Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require pillow>=10.0.0, catch drise failure due to low mask_res or num_masks #2181

Merged
merged 13 commits into from
Jul 26, 2023

Conversation

romanlutz
Copy link
Contributor

Description

In telemetry, we found that two exceptions were thrown. Firstly, drise throws ValueError when num_masks or mask_res are too low. This is caught by the try-except block and the _get_fail_str method subsequently runs into an AttributeError because it doesn't have a textsize method. This is due to a breaking change in v10.0.0 of Pillow.

This PR therefore

  • requires Pillow>=10.0.0 explicitly so that dependencies can't later accidentally downgrade us. We are already using Pillow==10.0.0 in our environments, so this isn't necessarily a change, just insurance.
  • uses the new method textbbox instead of textsize from Pillow which avoids the second exception
  • catches the ValueError from drise and provides a corresponding user error with informative message to the user.

A simple way to reproduce this is to take our dpv2 test cases and run test_object_detection_pipeline_rai.yaml with num_masks=1 and max_evals=10 (to make it faster).

Checklist

  • I have added screenshots above for all UI changes.
  • I have added e2e tests for all UI changes.
  • Documentation was updated if it was needed.

1 similar comment
1 similar comment
3 similar comments
1 similar comment
@codecov-commenter
Copy link

codecov-commenter commented Jul 25, 2023

Codecov Report

Merging #2181 (7b6576f) into main (8898e82) will decrease coverage by 0.26%.
Report is 3 commits behind head on main.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #2181      +/-   ##
==========================================
- Coverage   78.34%   78.09%   -0.26%     
==========================================
  Files          26       26              
  Lines        2152     2159       +7     
==========================================
  Hits         1686     1686              
- Misses        466      473       +7     
Flag Coverage Δ
unittests 78.09% <0.00%> (-0.26%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...responsibleai_vision/managers/explainer_manager.py 62.97% <0.00%> (-1.32%) ⬇️

@romanlutz romanlutz merged commit f880bcc into main Jul 26, 2023
107 of 108 checks passed
@romanlutz romanlutz deleted the romanlutz/pillow_fix branch July 26, 2023 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants