Skip to content

Removed setters for read-only properties - #9

Closed
radarhere wants to merge 8 commits into
Yay295:image_mode_size_delegationfrom
radarhere:image_mode_size_delegation
Closed

radarhere wants to merge 8 commits into
Yay295:image_mode_size_delegationfrom
radarhere:image_mode_size_delegation

Conversation

@radarhere

Copy link
Copy Markdown

Suggestion for python-pillow#7271, to fix the failing tests.

I've removed the setters for mode and size, restored some of the lines you removed, removed a duplicate @property and updated a BMP test and GifImagePlugin.

You have considered that the image might not be loaded yet, meaning that there is no self.im. I think you also need to consider seeking to another frame, but before, at which time that there will be a self.im but the size of the image should be the size of the tile that isn't loaded yet. As a result, I've also added a @property in ImageFile for size. I've also added one for mode.

With these changes, all tests now pass.

@Yay295

Yay295 commented Aug 2, 2023

Copy link
Copy Markdown
Owner

I can see that six tests fail (in my local environment) if they aren't, but I don't understand why setting _mode and _size in these two locations is necessary:
https://github.com/radarhere/Pillow/blob/400b216630bc86f2b1fff386469e68b4c8d48fc2/src/PIL/Image.py#L508-L512
https://github.com/radarhere/Pillow/blob/400b216630bc86f2b1fff386469e68b4c8d48fc2/src/PIL/Image.py#L694-L699
Since im is being set at the same time, the mode and size properties should be getting their data from im, so it shouldn't be necessary to also set _mode and _size.

@radarhere

Copy link
Copy Markdown
Author

It is because an ImagingCore instance with one dimension of zero is falsy.

>>> from PIL import Image
>>> "truthy" if Image.new("L", (0, 1)).im else "falsy"
'falsy'
>>> "truthy" if Image.new("L", (1, 0)).im else "falsy"
'falsy'
>>> "truthy" if Image.new("L", (1, 1)).im else "falsy"
'truthy'

This is producing results that you don't expect when running against self.im.mode if self.im else self._mode.

If you dislike this, then you can either change the self.im check to compare against None instead, or try and change how ImagingCore instances evaluate to booleans.

@Yay295
Yay295 force-pushed the image_mode_size_delegation branch from e311a46 to 919dbbe Compare August 5, 2023 20:02
@radarhere radarhere closed this Aug 5, 2023
@radarhere
radarhere deleted the image_mode_size_delegation branch August 5, 2023 20:38
radarhere pushed a commit that referenced this pull request Dec 24, 2023
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.

2 participants