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

Fix default image on image read failure #3073

Merged
merged 10 commits into from
Feb 13, 2023

Conversation

geoffreyangus
Copy link
Contributor

This PR fixes the default image used on image read failure. The default image is inserted after images are read, meaning that it bypasses the cast to np.float32 the default image was set to be np.uint8. This change ensures that it is also np.float32, and more importantly, that we have tests around default image to prevent this from happening again. The test mocks the image read function to simulate image read failure.

Copy link
Contributor

@jeffkinnison jeffkinnison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! That was a subtle issue, great job tracking it down!

@github-actions
Copy link

github-actions bot commented Feb 10, 2023

Unit Test Results

         6 files  ±  0           6 suites  ±0   5h 5m 54s ⏱️ + 13m 2s
  3 930 tests +  1    3 893 ✔️ +  1    37 💤 ±0  0 ±0 
11 737 runs   - 47  11 627 ✔️  - 46  110 💤  - 1  0 ±0 

Results for commit 4c5b045. ± Comparison against base commit 9662fe6.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@justinxzhao justinxzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one nit!

tests/integration_tests/utils.py Outdated Show resolved Hide resolved
@geoffreyangus geoffreyangus marked this pull request as ready for review February 13, 2023 18:49
@geoffreyangus geoffreyangus merged commit 8d7eaee into master Feb 13, 2023
@geoffreyangus geoffreyangus deleted the fix-read-image-failure-default-image branch February 13, 2023 20:01
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.

3 participants