-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Mitigate PIL Image.fromarray() mode deprecation #9150
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
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/9150
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 4ace920 with merge base b818d32 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
torchvision/utils.py
Outdated
|
||
PILLOW_VERSION = tuple(int(x) for x in PILLOW_VERSION_STRING.split(".")) |
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.
I stole this from our testing code. I'm doing this instead of adding a dependence on the packaging
module to parse the version string. If we'd rather add that dependence in order to avoid this code on the production side, I'm happy to do that as well.
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.
Agreed not to add packaging
for now.
For safety, let's have that inline within _Image_fromarray
. It's possible that some non-stable release versions won't have a .
, in which case the int()
call potentially fails.
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 the case that it fails, I'm going to propose we fall back to the old behavior since it's like more people will have the old PIL versions.
setup.py
Outdated
@@ -112,7 +112,7 @@ def get_dist(pkgname): | |||
|
|||
# Excluding 8.3.* because of https://github.com/pytorch/vision/issues/4934 | |||
# TODO remove <11.3 bound and address corresponding deprecation warnings |
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.
you might want to remove the "todo" line.
PS. I'm just a bystander that got affect by these tests failures.
# img = Image.fromarray(obj) | ||
# img = img.convert(mode) |
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.
Wellll that sucks. I was really hoping this would just be it. I personally don't even understand the claim that the mode can be inferred from the shape and dtype. Looking at the docs on mode https://pillow.readthedocs.io/en/stable/handbook/concepts.html#concept-modes there seem to be plenty of opportunity for collisions:
RGB (3x8-bit pixels, true color)
YCbCr (3x8-bit pixels, color video format)
LAB (3x8-bit pixels, the L*a*b color space)
HSV (3x8-bit pixels, Hue, Saturation, Value color space)
I took a brief look at python-pillow/Pillow#9018, apparently mode
has caused a lot of confusion in the past. But the collision is being acknowledged by the creation of python-pillow/Pillow#9063, which seems to only deprecate mode
for certain conversions. I don't know how it will affect us but it is not merged yet and won't be released before a while, so we still have to go for this complex workaround :(
Thank you for the fix @scotts ! Maybe we should add a comment to follow-up on this with a link to python-pillow/Pillow#9018 and python-pillow/Pillow#9063
Hey @scotts! You merged this PR, but no labels were added. |
Reviewed By: AntoineSimoulin Differential Revision: D79175040 fbshipit-source-id: 67a130dcdfe1c420bde815b2b5e52aa019275e04
Fixes #9135.
PIL version 11.3 deprecates the
mode
parameter toImage.fromarray()
: https://pillow.readthedocs.io/en/stable/releasenotes/11.3.0.html#image-fromarray-mode-parameter.The new behavior is that the mode is inferred from the type and shape of the data passed in. However, we depend on the
mode
infunctiona.to_pil_image()
andtransforms.ToPILImage()
to perform a conversion. That is, we expect that given the same data but different modes, we should get back different images. That behavior is no longer supported inImage.fromarray()
.This PR adds a utility function that figures out the version of PIL and calls
Image.fromarray()
for older versions of PIL, and callsImage.frombuffer()
for newer versions of PIL.Image.fromarray()
itself callsImage.frombuffer()
, andImage.frombuffer()
still does perform the conversion. How we do this is ugly, as we lift some of the actual implementation fromImage.fromarray()
; see the comment.The alternative approach of explicitly calling
Image.convert
does not work; we get very different pixel values back.cc @vfdev-5