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

BUG: avoid AttributeError when setting mode in Pillow v10.1 #1045

Merged
merged 13 commits into from Nov 7, 2023

Conversation

FirefoxMetzger
Copy link
Contributor

Closes: #1044

Pillow v10.1 removed the setter for Image.mode as this can have unintended consequences. This is done by making Image.mode a @property and keeping the mode in Image._mode.

While okay in general, it breaks our setup as there is currently no good way of setting 16-bit return types directly (which we want when reading 16-bit grayscale PNGs). As a result, this change breaks our current setup.

This PR introduces a "quick fix" by directly setting Image._mode instead of the previous Image.mode. I'm not fond of this solution, but until something better becomes available this is the only way I can think of to maintain our current behavior.

@FirefoxMetzger
Copy link
Contributor Author

Looks like there is not just a problem with Image.mode becoming read-only, but also some additions to pillow's GIF reader that cause test failures.

I'll have to set some time aside to figure out where/why this breaks.

@lagru
Copy link

lagru commented Oct 18, 2023

@FirefoxMetzger thanks for the quick reaction and working on this!

@FirefoxMetzger
Copy link
Contributor Author

@lagru I've merged a pin for pillow<10.1 for now (that one passed CI 🙃 ). As for a proper fix, I can pick up this PR again earliest tomorrow evening. However, chances are that I can't fully solve this and will need some feedback from the pillow devs before it can be properly resolved.

@lagru
Copy link

lagru commented Oct 18, 2023

No worries. We've applied the same temporary fix to scikit-image and are in no particular hurry. Please don't feel rushed. :)

@FirefoxMetzger
Copy link
Contributor Author

Looks like I finally managed to solve all the CI issues. Now it's only test coverage left.

I also want to check why we can now use "rawmodes" like "I;16" in PIL.Image.convert, which wasn't possible before but is now. I need to double-check if this internally promotes to 32-bit or reads straight into 16-bit.

@FirefoxMetzger FirefoxMetzger merged commit f58379c into imageio:master Nov 7, 2023
20 of 21 checks passed
@FirefoxMetzger FirefoxMetzger deleted the fix-pillow branch November 7, 2023 06:44
@FirefoxMetzger
Copy link
Contributor Author

FirefoxMetzger commented Nov 7, 2023

For future reference: Recent pillow versions no longer allow direct decoding of 16-bit grayscale PNGs. While we instruct Pillow to do so using the (now available) mode I;16 in Image.convert, this will internally decode the image as 32-bit and then convert to 16-bit in a copy.

I will create a ticket over at pillow and see if we can bring the old functionality back - perhaps in a different way - since it is quite useful and I would eventually like to use it in other places as well.

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.

Support for Pillow v10.1 (re: setting image.mode)
2 participants