Skip to content

Conversation

@radarhere
Copy link
Member

Aside from the obvious type hinting, this has two changes.

  1. Require fp in

    Pillow/src/PIL/ImageFile.py

    Lines 107 to 110 in 44c2ff3

    class ImageFile(Image.Image):
    """Base class for image file format handlers."""
    def __init__(self, fp=None, filename=None):

    It will become the stream that is used by plugins, so there's no reason for it to be None.

  2. Rather than allowing im.tile to be None

    self.tile = None

    Pillow/src/PIL/ImageFile.py

    Lines 178 to 185 in 44c2ff3

    def load(self) -> Image.core.PixelAccess | None:
    """Load image data based on tile list"""
    if self.tile is None:
    msg = "cannot load this image"
    raise OSError(msg)
    pixel = Image.Image.load(self)

    I have changed the check to
    if not self.tile and self._im is None:

    as it seems simpler from a type hinting perspective. If there is no tile to load, and no core image object already loaded, then this image cannot be loaded.

self.fp = fp
self.filename = filename
self.fp = cast(IO[bytes], fp)
self.filename = filename if filename is not None else ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be self.filename = filename or ""

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not exactly the same. This suggestion would mean that a filename of b"" would be unexpectedly changed to ""

>>> filename = b""
>>> filename if filename is not None else ""
b''
>>> filename = b""
>>> filename or ""
''

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I hadn't thought of that situation.

@hugovk hugovk merged commit 8aa1e92 into python-pillow:main Sep 4, 2024
@radarhere radarhere deleted the type_hint_imagefile branch September 4, 2024 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants