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 6 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
18 changes: 1 addition & 17 deletions docs/reference/ImageDraw.rst
Original file line number Diff line number Diff line change
Expand Up @@ -679,23 +679,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
41 changes: 3 additions & 38 deletions docs/reference/PixelAccess.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,42 +44,7 @@ Access using negative indexes is also possible. ::
-----------------------------

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

.. method:: __setitem__(self, xy, color):

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

: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.

.. method:: putpixel(self, xy, color):

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.

: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.
.. automethod:: PIL.Image.PixelAccess.__getitem__
.. automethod:: PIL.Image.PixelAccess.__setitem__
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__
62 changes: 49 additions & 13 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, cast
from typing import (

Check warning on line 44 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L44

Added line #L44 was not covered by tests
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.
Expand Down Expand Up @@ -482,6 +491,31 @@
# Implementation wrapper


class PixelAccess(Protocol):
def __getitem__(self, xy: tuple[int, int]) -> float | tuple[int, ...]:

Check warning on line 495 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L494-L495

Added lines #L494 - L495 were not covered by tests
"""
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()

Check warning on line 504 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L504

Added line #L504 was not covered by tests

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

Check warning on line 506 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L506

Added line #L506 was not covered by tests
"""
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()

Check warning on line 516 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L516

Added line #L516 was not covered by tests


class Image:
"""
This class represents an image object. To create
Expand Down Expand Up @@ -834,7 +868,7 @@
msg = "cannot decode image data"
raise ValueError(msg)

def load(self):
def load(self) -> PixelAccess | None:

Check warning on line 871 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L871

Added line #L871 was not covered by tests
"""
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 @@ -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`
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 @@ -876,6 +910,7 @@
if self.pyaccess:
return self.pyaccess
return self.im.pixel_access(self.readonly)
return None

Check warning on line 913 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L913

Added line #L913 was not covered by tests

def verify(self) -> None:
"""
Expand Down Expand Up @@ -1487,7 +1522,7 @@
self._exif._loaded = False
self.getexif()

def get_child_images(self):
def get_child_images(self) -> list[ImageFile.ImageFile]:

Check warning on line 1525 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L1525

Added line #L1525 was not covered by tests
child_images = []
exif = self.getexif()
ifds = []
Expand All @@ -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)

Check warning on line 1549 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L1549

Added line #L1549 was not covered by tests
self.fp.seek(thumbnail_offset)
data = self.fp.read(ifd.get(514))
fp = io.BytesIO(data)
Expand Down Expand Up @@ -1580,7 +1612,7 @@
or "transparency" in self.info
)

def apply_transparency(self):
def apply_transparency(self) -> None:

Check warning on line 1615 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L1615

Added line #L1615 was not covered by tests
"""
If a P mode image has a "transparency" key in the info dictionary,
remove the key and instead apply the transparency to the palette.
Expand All @@ -1592,6 +1624,7 @@
from . import ImagePalette

palette = self.getpalette("RGBA")
assert palette is not None

Check warning on line 1627 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L1627

Added line #L1627 was not covered by tests
transparency = self.info["transparency"]
if isinstance(transparency, bytes):
for i, alpha in enumerate(transparency):
Expand All @@ -1603,7 +1636,9 @@

del self.info["transparency"]

def getpixel(self, xy):
def getpixel(

Check warning on line 1639 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L1639

Added line #L1639 was not covered by tests
self, xy: tuple[SupportsInt, SupportsInt]
Copy link
Member

Choose a reason for hiding this comment

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

The test suite does provide a list for xy

def test_list(self) -> None:
im = hopper()
assert im.getpixel([0, 0]) == (20, 20, 70)

Also, why use SupportsInt, and not just int?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, why use SupportsInt, and not just int?

I wanted to reflect that this is passed to a C function that calls __int__ on the provided coordinates (unless they are an int or float):

Pillow/src/_imaging.c

Lines 1165 to 1170 in 114e017

if (PyLong_Check(value)) {
*x = PyLong_AS_LONG(value);
} else if (PyFloat_Check(value)) {
*x = (int)PyFloat_AS_DOUBLE(value);
} else {
PyObject *int_value = PyObject_CallMethod(value, "__int__", NULL);

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.

Expand Down Expand Up @@ -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:

Check warning on line 1905 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L1905

Added line #L1905 was not covered by tests
"""
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".
Expand Down Expand Up @@ -1914,6 +1949,7 @@
alpha = alpha.convert("L")
else:
# constant alpha
alpha = cast(int, alpha) # see python/typing#1013

Check warning on line 1952 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L1952

Added line #L1952 was not covered by tests
try:
self.im.fillband(band, alpha)
except (AttributeError, ValueError):
Expand Down Expand Up @@ -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:

Check warning on line 2016 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L2016

Added line #L2016 was not covered by tests
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
Expand Down Expand Up @@ -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]

Check warning on line 2056 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L2056

Added line #L2056 was not covered by tests
return self.im.putpixel(xy, value)

def remap_palette(self, dest_map, source_palette=None):
Expand Down
13 changes: 11 additions & 2 deletions src/PIL/ImageDraw.py
Original file line number Diff line number Diff line change
Expand Up @@ -898,9 +898,17 @@
return im, handler


def floodfill(image: Image.Image, xy, value, border=None, thresh=0) -> None:
def floodfill(

Check warning on line 901 in src/PIL/ImageDraw.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/ImageDraw.py#L901

Added line #L901 was not covered by tests
image: Image.Image,
xy: tuple[int, int],
value: float | tuple[int, ...],
border: float | tuple[int, ...] | None = None,
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 @@ -918,6 +926,7 @@
# based on an implementation by Eric S. Raymond
# amended by yo1995 @20180806
pixel = image.load()
assert pixel is not None

Check warning on line 929 in src/PIL/ImageDraw.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/ImageDraw.py#L929

Added line #L929 was not covered by tests
x, y = xy
try:
background = pixel[x, y]
Expand Down
15 changes: 11 additions & 4 deletions src/PIL/ImageGrab.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,17 @@
import subprocess
import sys
import tempfile
from typing import Union, cast

Check warning on line 25 in src/PIL/ImageGrab.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/ImageGrab.py#L25

Added line #L25 was not covered by tests

from . import Image


def grab(bbox=None, include_layered_windows=False, all_screens=False, xdisplay=None):
def grab(

Check warning on line 30 in src/PIL/ImageGrab.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/ImageGrab.py#L30

Added line #L30 was not covered by tests
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")
Expand All @@ -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)

Check warning on line 45 in src/PIL/ImageGrab.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/ImageGrab.py#L45

Added line #L45 was not covered by tests
im.load()
os.unlink(filepath)
if bbox:
Expand All @@ -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]

Check warning on line 72 in src/PIL/ImageGrab.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/ImageGrab.py#L72

Added line #L72 was not covered by tests
Copy link
Contributor Author

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.

try:
if not Image.core.HAVE_XCB:
msg = "Pillow was built without XCB support"
Expand All @@ -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]

Check warning on line 87 in src/PIL/ImageGrab.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/ImageGrab.py#L87

Added line #L87 was not covered by tests
im.load()
os.unlink(filepath)
if bbox:
Expand All @@ -94,7 +101,7 @@
return im


def grabclipboard():
def grabclipboard() -> Image.Image | list[str] | None:

Check warning on line 104 in src/PIL/ImageGrab.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/ImageGrab.py#L104

Added line #L104 was not covered by tests
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
Loading
Loading