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

Add various type annotations #8046

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

Conversation

srittau
Copy link

@srittau srittau commented May 7, 2024

Changes proposed in this pull request:

  • This PR add various type annotations, especially for the Image, ImageFont, ImageDraw, and _imagingft modules.

@nulano
Copy link
Contributor

nulano commented May 7, 2024

FWIW I have some WIP type annotations for ImageFont and _imagingft here: main...nulano:Pillow:types-imagefont

@srittau srittau marked this pull request as ready for review May 8, 2024 10:15
@srittau
Copy link
Author

srittau commented May 8, 2024

I think this is ready for review. The docs build might still fail, but I'd need help with that.

@srittau
Copy link
Author

srittau commented May 8, 2024

Also, the pypy3.10 failure seems unrelated?

@hugovk
Copy link
Member

hugovk commented May 8, 2024

Yeah, the PyPy one is a bit flaky.

@radarhere
Copy link
Member

The docs build might still fail, but I'd need help with that.

The error

docstring of PIL.Image.Image.transform:1: WARNING: py:class reference target not found: PIL.Image.GetDataTransform

is saying that the GetDataTransform doesn't appear in the documentation, so transform()'s description can't link to it.

The way to solve this is by updating docs/reference/Image.rst to include it, by adding

.. autoclass:: GetDataTransform
    :show-inheritance:

But given that our other protocols so far are SupportsGetMesh and SupportsArrayInterface, maybe this new protocol could be renamed SupportsGetData?

src/PIL/ImageFont.py Outdated Show resolved Hide resolved
@@ -1,3 +1,18 @@
from typing import Any

from typing_extensions import Buffer
Copy link
Member

Choose a reason for hiding this comment

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

Just pointing out that is adding typing_extensions as a requirement. Previous discussion about this can be seen in #7642

Copy link
Author

Choose a reason for hiding this comment

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

This is not super critical. We could use bytes | bytesarray | memoryview here, instead. This would exclude other types using the buffer protocol, but I'm not sure how common that is with Pillow. (And it can always be # type: ignored, of course.)

src/PIL/Image.py Outdated Show resolved Hide resolved
@@ -34,7 +34,7 @@ def transform(
self,
size: tuple[int, int],
image: Image.Image,
**options: dict[str, str | int | tuple[int, ...] | list[int]],
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain why you made this change?

Copy link
Author

Choose a reason for hiding this comment

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

The problem here is that options can be any of the remaining Image.transform arguments, i.e.:

        resample: int = Resampling.NEAREST,
        fill: int = 1,
        fillcolor: float | tuple[float, ...] | str | None 

dict is wrong in any case, but we could annotate it as float | tuple[float, ...] | str | None (with int being redundant with float), but that would imply that any argument could have that type. I see four alternatives:

  • Don't bother and use Any.
  • Use float | tuple[float, ...] | str | None as that would prevent some typing errors, but not all (e.g. fill=(1,2,3) would not be caught), at the cost of a slightly misleading type hints.
  • Just replace options with the actual forwarded arguments. (The option I would probably choose, as there are only three options.)
  • Use Unpack, which means we would need to introduce a new TypedDict for the arguments.

src/PIL/ImageFont.py Outdated Show resolved Hide resolved
srittau and others added 2 commits May 18, 2024 11:22
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
@radarhere
Copy link
Member

I've created srittau#2 with various suggestions.

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.

None yet

4 participants