-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Relaxed child images check to allow for libjpeg #6853
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
Conversation
Sorry, this was a false alarm! I got caught out by the fact that Sorry for the mix-up! Perhaps it would be possible to widen the skip of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other words, this PR is still necessary (or at least useful for us), but should be good enough with the following:
@@ -447,7 +447,7 @@ def test_get_child_images(self): | |||
ims = im.get_child_images() | |||
|
|||
assert len(ims) == 1 | |||
assert_image_equal_tofile(ims[0], "Tests/images/flower_thumbnail.png") | |||
assert_image_similar_tofile(ims[0], "Tests/images/flower_thumbnail.png", 2.1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert_image_similar_tofile(ims[0], "Tests/images/flower_thumbnail.png", 2.1) | |
assert_image_similar_tofile(ims[0], "Tests/images/flower_thumbnail.png", 1.2) |
When I test with libjpeg 9e, I also found a difference of 2.0033 - https://github.com/radarhere/pillow-wheels/actions/runs/3821343292/jobs/6500396156 If we're going to change this to improve support, might as well change it to support more libjpeg environments in general. |
Our GitHub Actions uses libjpeg turbo 2.1 and passes - https://github.com/python-pillow/Pillow/actions/runs/3819974382/jobs/6497949271#step:8:39 If you'd like that to be investigated further, please open a new issue. |
Yeah, I should not have responded so quickly (but wanted to avoid an inadvertent merge in that moment). The 2.1 remains necessary on osx, where Sorry for the back and forth 😑 |
Fair enough. Since it's just a minor accuracy violation I'm going to skip that test for our builds. |
Reported in conda-forge/pillow-feedstock#128 (comment)
test_get_child_images()
in test_file_jpeg.py is failing with libjpeg. This PR relaxes the check to test for a similarity of 2.1, instead of equality. I have confirmed this in pillow-wheels with libjpeg 9e.