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

Added type hints for PixelAccess related methods and others #8032

Merged
merged 21 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Tests/test_imagegrab.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ def test_grabclipboard_file(self) -> None:
p.communicate()

im = ImageGrab.grabclipboard()
assert isinstance(im, list)
assert len(im) == 1
assert os.path.samefile(im[0], "Tests/images/hopper.gif")

Expand All @@ -105,6 +106,7 @@ def test_grabclipboard_png(self) -> None:
p.communicate()

im = ImageGrab.grabclipboard()
assert isinstance(im, Image.Image)
assert_image_equal_tofile(im, "Tests/images/hopper.png")

@pytest.mark.skipif(
Expand All @@ -120,6 +122,7 @@ def test_grabclipboard_wl_clipboard(self, ext: str) -> None:
with open(image_path, "rb") as fp:
subprocess.call(["wl-copy"], stdin=fp)
im = ImageGrab.grabclipboard()
assert isinstance(im, Image.Image)
assert_image_equal_tofile(im, image_path)

@pytest.mark.skipif(
Expand Down
18 changes: 1 addition & 17 deletions docs/reference/ImageDraw.rst
Original file line number Diff line number Diff line change
Expand Up @@ -691,23 +691,7 @@ Methods
:param hints: An optional list of hints.
:returns: A (drawing context, drawing resource factory) tuple.

.. py:method:: floodfill(image, xy, value, border=None, thresh=0)

.. warning:: This method is experimental.

Fills a bounded region with a given color.

:param image: Target image.
:param xy: Seed position (a 2-item coordinate tuple).
:param value: Fill color.
:param border: Optional border value. If given, the region consists of
pixels with a color different from the border color. If not given,
the region consists of pixels having the same color as the seed
pixel.
:param thresh: Optional threshold value which specifies a maximum
tolerable difference of a pixel value from the 'background' in
order for it to be replaced. Useful for filling regions of non-
homogeneous, but similar, colors.
.. autofunction:: PIL.ImageDraw.floodfill

.. _BCP 47 language code: https://www.w3.org/International/articles/language-tags/
.. _OpenType docs: https://learn.microsoft.com/en-us/typography/opentype/spec/featurelist
37 changes: 9 additions & 28 deletions docs/reference/PixelAccess.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,42 +44,23 @@ Access using negative indexes is also possible. ::
-----------------------------

.. class:: PixelAccess
:canonical: PIL.Image.core.PixelAccess

.. method:: __setitem__(self, xy, color):
.. method:: __getitem__(self, xy: tuple[int, int]) -> float | tuple[int, ...]

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)

.. method:: __getitem__(self, xy):

Returns the pixel at x,y. The pixel is returned as a single
value for single band images or a tuple for multiple band
images
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.
pixel values for multiband images.

.. method:: putpixel(self, xy, color):
.. method:: __setitem__(self, xy: tuple[int, int], color: float | tuple[int, ...]) -> None
radarhere marked this conversation as resolved.
Show resolved Hide resolved

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. In addition to this, RGB and RGBA tuples
are accepted for P and PA images.
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)

.. method:: getpixel(self, xy):

Returns the pixel at x,y. The pixel is returned as a single
value for single band images or a tuple for multiple 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.
:param color: The pixel value according to its mode,
e.g. tuple (r, g, b) for RGB mode.
1 change: 1 addition & 0 deletions docs/reference/PyAccess.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,4 @@ Access using negative indexes is also possible. ::

.. autoclass:: PIL.PyAccess.PyAccess()
:members:
:special-members: __getitem__, __setitem__
47 changes: 33 additions & 14 deletions src/PIL/Image.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, Tuple, cast
from typing import (
IO,
TYPE_CHECKING,
Any,
Literal,
Protocol,
Sequence,
Tuple,
cast,
)

# VERSION was removed in Pillow 6.0.0.
# PILLOW_VERSION was removed in Pillow 9.0.0.
Expand Down Expand Up @@ -218,7 +227,7 @@ class Quantize(IntEnum):
# Registries

if TYPE_CHECKING:
from . import ImageFile
from . import ImageFile, PyAccess
ID: list[str] = []
OPEN: dict[
str,
Expand Down Expand Up @@ -863,7 +872,7 @@ def frombytes(self, data: bytes, decoder_name: str = "raw", *args) -> None:
msg = "cannot decode image data"
raise ValueError(msg)

def load(self):
def load(self) -> core.PixelAccess | PyAccess.PyAccess | None:
"""
Allocates storage for the image and loads the pixel data. In
normal cases, you don't need to call this method, since the
Expand All @@ -876,7 +885,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`
Copy link
Member

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?

Copy link
Member

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.

Copy link
Member

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 return core.PixelAccess | PyAccess.PyAccess | None instead.

Copy link
Contributor Author

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.

"""
if self.im is not None and self.palette and self.palette.dirty:
# realize palette
Expand Down Expand Up @@ -905,6 +914,7 @@ def load(self):
if self.pyaccess:
return self.pyaccess
return self.im.pixel_access(self.readonly)
return None

def verify(self) -> None:
"""
Expand Down Expand Up @@ -1092,7 +1102,10 @@ def convert_transparency(m, v):
del new_im.info["transparency"]
if trns is not None:
try:
new_im.info["transparency"] = new_im.palette.getcolor(trns, new_im)
new_im.info["transparency"] = new_im.palette.getcolor(
cast(Tuple[int, int, int], trns), # trns was converted to RGB
new_im,
)
except Exception:
# if we can't make a transparent color, don't leave the old
# transparency hanging around to mess us up.
Expand Down Expand Up @@ -1142,7 +1155,10 @@ def convert_transparency(m, v):
if trns is not None:
if new_im.mode == "P":
try:
new_im.info["transparency"] = new_im.palette.getcolor(trns, new_im)
new_im.info["transparency"] = new_im.palette.getcolor(
cast(Tuple[int, int, int], trns), # trns was converted to RGB
new_im,
)
except ValueError as e:
del new_im.info["transparency"]
if str(e) != "cannot allocate more than 256 colors":
Expand Down Expand Up @@ -1633,7 +1649,9 @@ def apply_transparency(self) -> None:

del self.info["transparency"]

def getpixel(self, xy):
def getpixel(
self, xy: tuple[int, int] | list[int]
) -> float | tuple[int, ...] | None:
"""
Returns the pixel value at a given position.

Expand Down Expand Up @@ -1902,15 +1920,14 @@ def point(self, data):
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".
The new layer must be either "L" or "1".

:param alpha: The new alpha layer. This can either be an "L" or "1"
image having the same size as this image, or an integer or
other color value.
image having the same size as this image, or an integer.
"""

self._ensure_mutable()
Expand Down Expand Up @@ -1949,6 +1966,7 @@ def putalpha(self, alpha):
alpha = alpha.convert("L")
else:
# constant alpha
alpha = cast(int, alpha) # see python/typing#1013
try:
self.im.fillband(band, alpha)
except (AttributeError, ValueError):
Expand Down Expand Up @@ -2014,7 +2032,9 @@ 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, ...] | list[int]
) -> None:
"""
Modifies the pixel at the given position. The color is given as
a single numerical value for single-band images, and a tuple for
Expand Down Expand Up @@ -2052,9 +2072,8 @@ def putpixel(self, xy, value):
if self.mode == "PA":
alpha = value[3] if len(value) == 4 else 255
value = value[:3]
value = self.palette.getcolor(value, self)
if self.mode == "PA":
value = (value, alpha)
palette_index = self.palette.getcolor(value, self)
value = (palette_index, alpha) if self.mode == "PA" else palette_index
return self.im.putpixel(xy, value)

def remap_palette(self, dest_map, source_palette=None):
Expand Down
5 changes: 4 additions & 1 deletion src/PIL/ImageDraw.py
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,9 @@ def floodfill(
thresh: float = 0,
) -> None:
"""
(experimental) Fills a bounded region with a given color.
.. warning:: This method is experimental.

Fills a bounded region with a given color.

:param image: Target image.
:param xy: Seed position (a 2-item coordinate tuple). See
Expand All @@ -944,6 +946,7 @@ def floodfill(
# based on an implementation by Eric S. Raymond
# amended by yo1995 @20180806
pixel = image.load()
assert pixel is not None
x, y = xy
try:
background = pixel[x, y]
Expand Down
16 changes: 12 additions & 4 deletions src/PIL/ImageGrab.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@
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:
im: Image.Image
if xdisplay is None:
if sys.platform == "darwin":
fh, filepath = tempfile.mkstemp(".png")
Expand Down Expand Up @@ -63,14 +69,16 @@ 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
# Cast to Optional[str] needed for Windows and macOS.
display_name: str | None = xdisplay
try:
if not Image.core.HAVE_XCB:
msg = "Pillow was built without XCB support"
raise OSError(msg)
size, data = Image.core.grabscreen_x11(xdisplay)
size, data = Image.core.grabscreen_x11(display_name)
except OSError:
if (
xdisplay is None
display_name is None
and sys.platform not in ("darwin", "win32")
and shutil.which("gnome-screenshot")
):
Expand All @@ -94,7 +102,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:
Copy link
Member

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not opposed to changing this to ImageFile, but would that unnecessarily lock us in to a given return type?
Or can we change return types without a major version bump and deprecation period?

Copy link
Member

Choose a reason for hiding this comment

The 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 ImageFile instead of an Image. So I think either form is fine.

if sys.platform == "darwin":
fh, filepath = tempfile.mkstemp(".png")
os.close(fh)
Expand Down
17 changes: 11 additions & 6 deletions src/PIL/PyAccess.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ def __init__(self, img: Image.Image, readonly: bool = False) -> None:
def _post_init(self) -> None:
pass

def __setitem__(self, xy, color):
def __setitem__(
self,
xy: tuple[int, int] | list[int],
color: float | tuple[int, ...] | list[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
Expand Down Expand Up @@ -107,13 +111,12 @@ def __setitem__(self, xy, color):
if self._im.mode == "PA":
alpha = color[3] if len(color) == 4 else 255
color = color[:3]
color = self._palette.getcolor(color, self._img)
if self._im.mode == "PA":
color = (color, alpha)
palette_index = self._palette.getcolor(color, self._img)
color = (palette_index, alpha) if self._im.mode == "PA" else palette_index

return self.set_pixel(x, y, color)

def __getitem__(self, xy: tuple[int, int]) -> float | tuple[int, ...]:
def __getitem__(self, xy: tuple[int, int] | list[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 multiple band
Expand Down Expand Up @@ -145,7 +148,9 @@ def check_xy(self, xy: tuple[int, int]) -> tuple[int, int]:
def get_pixel(self, x: int, y: int) -> float | tuple[int, ...]:
raise NotImplementedError()

def set_pixel(self, x: int, y: int, color: float | tuple[int, ...]) -> None:
def set_pixel(
self, x: int, y: int, color: float | tuple[int, ...] | list[int]
) -> None:
raise NotImplementedError()


Expand Down
5 changes: 4 additions & 1 deletion src/PIL/_imaging.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ class ImagingDraw:
def __getattr__(self, name: str) -> Any: ...

class PixelAccess:
def __getattr__(self, name: str) -> Any: ...
def __getitem__(self, xy: tuple[int, int]) -> float | tuple[int, ...]: ...
def __setitem__(
self, xy: tuple[int, int], color: float | tuple[int, ...]
) -> None: ...

def font(image, glyphdata: bytes) -> ImagingFont: ...
def __getattr__(name: str) -> Any: ...
Loading