-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Changes from 6 commits
5f805c3
b60b606
a304fd5
74b87ae
c2cb944
007caae
b2ce2f6
6affb12
c1f10c1
7e14364
eb56f3e
31a8da4
2a2033e
2381103
0a2baab
20ce7ad
32264a1
b64847e
ded4045
d0d53d4
ab18395
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -41,7 +41,16 @@ | |||||||||||||||||||
from collections.abc import Callable, MutableMapping | ||||||||||||||||||||
from enum import IntEnum | ||||||||||||||||||||
from types import ModuleType | ||||||||||||||||||||
from typing import IO, TYPE_CHECKING, Any, Literal, Protocol, Sequence, cast | ||||||||||||||||||||
from typing import ( | ||||||||||||||||||||
IO, | ||||||||||||||||||||
TYPE_CHECKING, | ||||||||||||||||||||
Any, | ||||||||||||||||||||
Literal, | ||||||||||||||||||||
Protocol, | ||||||||||||||||||||
Sequence, | ||||||||||||||||||||
SupportsInt, | ||||||||||||||||||||
cast, | ||||||||||||||||||||
) | ||||||||||||||||||||
|
||||||||||||||||||||
# VERSION was removed in Pillow 6.0.0. | ||||||||||||||||||||
# PILLOW_VERSION was removed in Pillow 9.0.0. | ||||||||||||||||||||
|
@@ -482,6 +491,31 @@ | |||||||||||||||||||
# Implementation wrapper | ||||||||||||||||||||
|
||||||||||||||||||||
|
||||||||||||||||||||
class PixelAccess(Protocol): | ||||||||||||||||||||
def __getitem__(self, xy: tuple[int, int]) -> float | tuple[int, ...]: | ||||||||||||||||||||
""" | ||||||||||||||||||||
Returns the pixel at x,y. The pixel is returned as a single | ||||||||||||||||||||
value for single band images or a tuple for multi-band images. | ||||||||||||||||||||
|
||||||||||||||||||||
:param xy: The pixel coordinate, given as (x, y). | ||||||||||||||||||||
:returns: a pixel value for single band images, a tuple of | ||||||||||||||||||||
pixel values for multiband images. | ||||||||||||||||||||
""" | ||||||||||||||||||||
raise NotImplementedError() | ||||||||||||||||||||
|
||||||||||||||||||||
def __setitem__(self, xy: tuple[int, int], color: float | tuple[int, ...]) -> None: | ||||||||||||||||||||
""" | ||||||||||||||||||||
Modifies the pixel at x,y. The color is given as a single | ||||||||||||||||||||
numerical value for single band images, and a tuple for | ||||||||||||||||||||
multi-band images. | ||||||||||||||||||||
|
||||||||||||||||||||
:param xy: The pixel coordinate, given as (x, y). | ||||||||||||||||||||
:param color: The pixel value according to its mode, | ||||||||||||||||||||
e.g. tuple (r, g, b) for RGB mode. | ||||||||||||||||||||
""" | ||||||||||||||||||||
raise NotImplementedError() | ||||||||||||||||||||
|
||||||||||||||||||||
|
||||||||||||||||||||
class Image: | ||||||||||||||||||||
""" | ||||||||||||||||||||
This class represents an image object. To create | ||||||||||||||||||||
|
@@ -834,7 +868,7 @@ | |||||||||||||||||||
msg = "cannot decode image data" | ||||||||||||||||||||
raise ValueError(msg) | ||||||||||||||||||||
|
||||||||||||||||||||
def load(self): | ||||||||||||||||||||
def load(self) -> PixelAccess | None: | ||||||||||||||||||||
""" | ||||||||||||||||||||
Allocates storage for the image and loads the pixel data. In | ||||||||||||||||||||
normal cases, you don't need to call this method, since the | ||||||||||||||||||||
|
@@ -847,7 +881,7 @@ | |||||||||||||||||||
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` | ||||||||||||||||||||
""" | ||||||||||||||||||||
if self.im is not None and self.palette and self.palette.dirty: | ||||||||||||||||||||
# realize palette | ||||||||||||||||||||
|
@@ -876,6 +910,7 @@ | |||||||||||||||||||
if self.pyaccess: | ||||||||||||||||||||
return self.pyaccess | ||||||||||||||||||||
return self.im.pixel_access(self.readonly) | ||||||||||||||||||||
return None | ||||||||||||||||||||
|
||||||||||||||||||||
def verify(self) -> None: | ||||||||||||||||||||
""" | ||||||||||||||||||||
|
@@ -1487,7 +1522,7 @@ | |||||||||||||||||||
self._exif._loaded = False | ||||||||||||||||||||
self.getexif() | ||||||||||||||||||||
|
||||||||||||||||||||
def get_child_images(self): | ||||||||||||||||||||
def get_child_images(self) -> list[ImageFile.ImageFile]: | ||||||||||||||||||||
child_images = [] | ||||||||||||||||||||
exif = self.getexif() | ||||||||||||||||||||
ifds = [] | ||||||||||||||||||||
|
@@ -1511,10 +1546,7 @@ | |||||||||||||||||||
fp = self.fp | ||||||||||||||||||||
thumbnail_offset = ifd.get(513) | ||||||||||||||||||||
if thumbnail_offset is not None: | ||||||||||||||||||||
try: | ||||||||||||||||||||
thumbnail_offset += self._exif_offset | ||||||||||||||||||||
except AttributeError: | ||||||||||||||||||||
pass | ||||||||||||||||||||
thumbnail_offset += getattr(self, "_exif_offset", 0) | ||||||||||||||||||||
self.fp.seek(thumbnail_offset) | ||||||||||||||||||||
data = self.fp.read(ifd.get(514)) | ||||||||||||||||||||
fp = io.BytesIO(data) | ||||||||||||||||||||
|
@@ -1580,7 +1612,7 @@ | |||||||||||||||||||
or "transparency" in self.info | ||||||||||||||||||||
) | ||||||||||||||||||||
|
||||||||||||||||||||
def apply_transparency(self): | ||||||||||||||||||||
def apply_transparency(self) -> None: | ||||||||||||||||||||
""" | ||||||||||||||||||||
If a P mode image has a "transparency" key in the info dictionary, | ||||||||||||||||||||
remove the key and instead apply the transparency to the palette. | ||||||||||||||||||||
|
@@ -1592,6 +1624,7 @@ | |||||||||||||||||||
from . import ImagePalette | ||||||||||||||||||||
|
||||||||||||||||||||
palette = self.getpalette("RGBA") | ||||||||||||||||||||
assert palette is not None | ||||||||||||||||||||
transparency = self.info["transparency"] | ||||||||||||||||||||
if isinstance(transparency, bytes): | ||||||||||||||||||||
for i, alpha in enumerate(transparency): | ||||||||||||||||||||
|
@@ -1603,7 +1636,9 @@ | |||||||||||||||||||
|
||||||||||||||||||||
del self.info["transparency"] | ||||||||||||||||||||
|
||||||||||||||||||||
def getpixel(self, xy): | ||||||||||||||||||||
def getpixel( | ||||||||||||||||||||
self, xy: tuple[SupportsInt, SupportsInt] | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test suite does provide a list for Pillow/Tests/test_image_access.py Lines 217 to 219 in 95a69ec
Also, why use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I wanted to reflect that this is passed to a C function that calls Lines 1165 to 1170 in 114e017
However, I see now that this doesn't work for self.pyaccess which requires an actual int and rejects a float.
|
||||||||||||||||||||
) -> float | tuple[int, ...] | None: | ||||||||||||||||||||
""" | ||||||||||||||||||||
Returns the pixel value at a given position. | ||||||||||||||||||||
|
||||||||||||||||||||
|
@@ -1867,7 +1902,7 @@ | |||||||||||||||||||
lut = [round(i) for i in lut] | ||||||||||||||||||||
return self._new(self.im.point(lut, mode)) | ||||||||||||||||||||
|
||||||||||||||||||||
def putalpha(self, alpha): | ||||||||||||||||||||
def putalpha(self, alpha: Image | int) -> None: | ||||||||||||||||||||
""" | ||||||||||||||||||||
Adds or replaces the alpha layer in this image. If the image | ||||||||||||||||||||
does not have an alpha layer, it's converted to "LA" or "RGBA". | ||||||||||||||||||||
|
@@ -1914,6 +1949,7 @@ | |||||||||||||||||||
alpha = alpha.convert("L") | ||||||||||||||||||||
else: | ||||||||||||||||||||
# constant alpha | ||||||||||||||||||||
alpha = cast(int, alpha) # see python/typing#1013 | ||||||||||||||||||||
try: | ||||||||||||||||||||
self.im.fillband(band, alpha) | ||||||||||||||||||||
except (AttributeError, ValueError): | ||||||||||||||||||||
|
@@ -1977,7 +2013,7 @@ | |||||||||||||||||||
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: | ||||||||||||||||||||
radarhere marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||
""" | ||||||||||||||||||||
Modifies the pixel at the given position. The color is given as | ||||||||||||||||||||
a single numerical value for single-band images, and a tuple for | ||||||||||||||||||||
|
@@ -2017,7 +2053,7 @@ | |||||||||||||||||||
value = value[:3] | ||||||||||||||||||||
value = self.palette.getcolor(value, self) | ||||||||||||||||||||
if self.mode == "PA": | ||||||||||||||||||||
value = (value, alpha) | ||||||||||||||||||||
value = (value, alpha) # type: ignore[assignment] | ||||||||||||||||||||
return self.im.putpixel(xy, value) | ||||||||||||||||||||
|
||||||||||||||||||||
def remap_palette(self, dest_map, source_palette=None): | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,11 +22,17 @@ | |
import subprocess | ||
import sys | ||
import tempfile | ||
from typing import Union, cast | ||
|
||
from . import Image | ||
|
||
|
||
def grab(bbox=None, include_layered_windows=False, all_screens=False, xdisplay=None): | ||
def grab( | ||
bbox: tuple[int, int, int, int] | None = None, | ||
include_layered_windows: bool = False, | ||
all_screens: bool = False, | ||
xdisplay: str | None = None, | ||
) -> Image.Image: | ||
if xdisplay is None: | ||
if sys.platform == "darwin": | ||
fh, filepath = tempfile.mkstemp(".png") | ||
|
@@ -36,7 +42,7 @@ | |
left, top, right, bottom = bbox | ||
args += ["-R", f"{left},{top},{right-left},{bottom-top}"] | ||
subprocess.call(args + ["-x", filepath]) | ||
im = Image.open(filepath) | ||
im: Image.Image = Image.open(filepath) | ||
im.load() | ||
os.unlink(filepath) | ||
if bbox: | ||
|
@@ -63,6 +69,7 @@ | |
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 commentThe reason will be displayed to describe this comment to others. Learn more. This cast is required on Windows and macOS. |
||
try: | ||
if not Image.core.HAVE_XCB: | ||
msg = "Pillow was built without XCB support" | ||
|
@@ -77,7 +84,7 @@ | |
fh, filepath = tempfile.mkstemp(".png") | ||
os.close(fh) | ||
subprocess.call(["gnome-screenshot", "-f", filepath]) | ||
im = Image.open(filepath) | ||
im: Image.Image = Image.open(filepath) # type: ignore[no-redef, unused-ignore] | ||
im.load() | ||
os.unlink(filepath) | ||
if bbox: | ||
|
@@ -94,7 +101,7 @@ | |
return im | ||
|
||
|
||
def grabclipboard(): | ||
def grabclipboard() -> Image.Image | list[str] | None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not opposed to changing this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hope that we can change return types without deprecation, but I can appreciate that we could be a bit forwards-compatible here. There's also no reason for the user to expect |
||
if sys.platform == "darwin": | ||
fh, filepath = tempfile.mkstemp(".png") | ||
os.close(fh) | ||
|
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.
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.
I think that after 10.4.0, we'll have no need for the protocol - we can just use
core.PixelAccess
. If so, then it's not helpful to add a public protocol only to remove it in the next version. I've created nulano#39 to returncore.PixelAccess | PyAccess.PyAccess | None
instead.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.
Fair enough, I've pushed a similar commit, see comment in nulano#39.