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

Moved get_child_images() to ImageFile #8689

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

radarhere
Copy link
Member

This is part of #8362 - I'm hoping to break down that PR into easier-to-review chunks.

get_child_images() relies on self.fp, and since that only exists for an ImageFile and not an Image, I suggest moving it there. This isn't a strictly necessary change, so feel free to disagree.

After that, I've also made changes to fix two mypy errors. One is asserting that fp is not None, like #8617. The other is asserting that an Exif tag is an integer, to fix

src/PIL/ImageFile.py:238: error: Argument 1 to "read" of "IO" has incompatible type "Optional[Any]"; expected "int" [arg-type]
data = self.fp.read(ifd.get(ExifTags.Base.JpegIFByteCount))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@hugovk
Copy link
Member

hugovk commented Jan 16, 2025

get_child_images() relies on self.fp, and since that only exists for an ImageFile and not an Image, I suggest moving it there. This isn't a strictly necessary change, so feel free to disagree.

I wonder, are there some circumstances where this could be a breaking change?

@radarhere
Copy link
Member Author

Images that are not ImageFiles

>>> from PIL import Image
>>> im = Image.new("RGB", (1, 1))
>>> im.get_child_images()
[]

would start failing.

>>> im.get_child_images()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'Image' object has no attribute 'get_child_images'

However, I would suggest that is more helpful than an apparently-successful empty list, since an Image by itself will never return any child images - for example, if I open an image as an ImageFile, and then copy() to turn it into an Image, there is an error.

>>> from PIL import Image
>>> im = Image.open("Tests/images/flower.jpg")
>>> im.get_child_images()
[<PIL.JpegImagePlugin.JpegImageFile image mode=RGB size=160x120 at 0x103ACA970>]
>>> im.copy().get_child_images()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "PIL/Image.py", line 1574, in get_child_images
    current_offset = self.fp.tell()
AttributeError: 'Image' object has no attribute 'fp'

That would now change to

>>> im.copy().get_child_images()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'Image' object has no attribute 'get_child_images'

which again, I think is more helpful.

@hugovk
Copy link
Member

hugovk commented Jan 16, 2025

Yes okay, if you'd like to go ahead with this, I suggest a deprecation cycle, so we don't break things without warning.

@radarhere radarhere added the Deprecation Feature that will be removed in the future label Jan 17, 2025
@radarhere
Copy link
Member Author

Ok, I've deprecated the functionality, to be removed in Pillow 13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecation Feature that will be removed in the future
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants