Skip to content

Conversation

@DominicOram
Copy link

printing), use ``list(im.getdata())``.
recommends that people use list(im.getdata()) for e.g. printing data. This works fine in production but type checkers (such as pyright) do not like it as ImagingCore does not expose an __iter__. This adds that so that the type checking is happy.

Notes

  • It would be better to not say that the __iter__ returns Any but I do not know the codebase well enough to know if there are variations in what types it might return. Any is sufficient to fix the type checker for my usecase

@DiamondJoseph
Copy link

Just guessing from the other methods, would it be

from collections.abc import Iterator
...

    def __getitem__(self, index: int) -> float: ...
    def __iter__(self) -> Iterator[float]: ...

Or are other elements returned also?

@radarhere
Copy link
Member

My suggestion would be float | tuple[int, ...] | None to mirror

Pillow/src/PIL/Image.py

Lines 1631 to 1633 in 5122c83

def getpixel(
self, xy: tuple[int, int] | list[int]
) -> float | tuple[int, ...] | None:

Regarding the first code you found

def __getitem__(self, index: int) -> float: ...

I think this should actually be updated as well.

def __iter__(self) -> Iterator[float]: ...

I don't know where this came from.

@tpoliaw
Copy link

tpoliaw commented Oct 16, 2025

>>> ic = im.getdata()
>>> ic.__iter__()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'ImagingCore' object has no attribute '__iter__'. Did you mean: '__str__'?

I think the problem is in pyright/pyshed more than pillow. The ImagingCore type has no __iter__ method so while the PR fixes the linting, it's not really accurate. Adding __len__ to the type-stub would be more correct but still wouldn't fix the problem here because the typing on the list method is too restrictive which (I think?) comes from typeshed.

A workaround that passes type-checking could be list(iter(im.getdata()))

@tpoliaw
Copy link

tpoliaw commented Oct 16, 2025

For completeness, I found this issue in typeshed and the upshot was that supporting __getitem__ iterators wasn't feasible and calling code should use iter.

@DominicOram
Copy link
Author

Ok, in which case I will change the comment in

printing), use ``list(im.getdata())``.
to suggest list(iter(im.getdata())) as it better matches the type

@DominicOram DominicOram force-pushed the fix_imagingcore_typing branch from db20e21 to fcc09af Compare October 16, 2025 13:25
@DominicOram DominicOram changed the title Add __iter__ type hint to ImagingCore Edit getdata to recommend iterating in a type-correct way Oct 16, 2025
new_im.info["transparency"] = new_im.palette.getcolor(
cast(tuple[int, ...], trns), new_im # trns was converted to RGB
cast(tuple[int, ...], trns),
new_im, # trns was converted to RGB
Copy link
Member

Choose a reason for hiding this comment

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

What was your thinking behind this reformatting?

def __arrow_c_array__(
self, requested_schema: "PyCapsule" = None # type: ignore[name-defined] # noqa: F821, UP037
self,
requested_schema: "PyCapsule" = None, # type: ignore[name-defined] # noqa: F821, UP037
Copy link
Member

Choose a reason for hiding this comment

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

What was your thinking behind this reformatting?

@radarhere
Copy link
Member

radarhere commented Oct 20, 2025

When I test, I don't find any issue with list(im.getdata()) in mypy.

I'm a bit reluctant to tell everyone to change their code in this way - not everyone uses type checkers, and of that subset, only a third use pyright.

A simpler alternative for pyright might actually be iterating over im.tobytes().

Others may disagree with me, but if you really think there is a problem here, I'd actually be more interested in deprecating getdata() altogether.

return self.im # could be abused

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.

4 participants