-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Added type hints for PixelAccess related methods and others #8032
base: main
Are you sure you want to change the base?
Conversation
@@ -63,6 +69,7 @@ def grab(bbox=None, include_layered_windows=False, all_screens=False, xdisplay=N | |||
left, top, right, bottom = bbox | |||
im = im.crop((left - x0, top - y0, right - x0, bottom - y0)) | |||
return im | |||
xdisplay = cast(Union[str, None], xdisplay) # type: ignore[redundant-cast, unused-ignore] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This cast is required on Windows and macOS.
src/PIL/PyAccess.py
Outdated
|
||
|
||
if TYPE_CHECKING: | ||
from . import Image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You didn't want to put this import at the top of the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For lighter/quicker imports when not type checking, and potentially avoid circular imports?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The convention so far has been to put the TYPE_CHECKING
imports just before their use - OPEN
, Exif
, ImageFileDirectory_v2
and PdfDict
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For lighter/quicker imports when not type checking, and potentially avoid circular imports?
I hadn't realized mypy has no problem dealing with circular imports. I've now moved the import near the top (still with the TYPE_CHECKING
check).
@@ -847,7 +881,7 @@ def load(self): | |||
operations. See :ref:`file-handling` for more information. | |||
|
|||
:returns: An image access object. | |||
:rtype: :ref:`PixelAccess` or :py:class:`PIL.PyAccess` | |||
:rtype: :py:class:`.PixelAccess` or :py:class:`.PyAccess` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My initial reaction to this was to feel confused , because the return type is PixelAccess or None, but this says PixelAccess or PyAccess.
Of course, the return type uses PixelAccess the protocol, not the class. But should the protocol have a different name perhaps to avoid confusion? SupportsPixelAccess
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be aware that this will become simpler after Pillow 10.4.0, as PyAccess will end deprecation and be removed.
@@ -94,7 +101,7 @@ def grab(bbox=None, include_layered_windows=False, all_screens=False, xdisplay=N | |||
return im | |||
|
|||
|
|||
def grabclipboard(): | |||
def grabclipboard() -> Image.Image | list[str] | None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be ImageFile.ImageFile
, rather than Image.Image
@@ -1977,7 +2013,7 @@ def putpalette(self, data, rawmode="RGB") -> None: | |||
self.palette.mode = "RGB" | |||
self.load() # install new palette | |||
|
|||
def putpixel(self, xy, value): | |||
def putpixel(self, xy: tuple[int, int], value: float | tuple[int, ...]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would appear value
can also be a list.
Line 2035 in c757439
and isinstance(value, (list, tuple)) |
No description provided.