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

fromarray: add type hints #7936

Merged
merged 9 commits into from
Apr 17, 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
10 changes: 10 additions & 0 deletions Tests/test_image_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,16 @@ def test(mode: str) -> tuple[str, tuple[int, int], bool]:
Image.fromarray(wrapped)


def test_fromarray_strides_without_tobytes() -> None:
class Wrapper:
def __init__(self, arr_params: dict[str, Any]) -> None:
self.__array_interface__ = arr_params

with pytest.raises(ValueError):
wrapped = Wrapper({"shape": (1, 1), "strides": (1, 1)})
Image.fromarray(wrapped, "L")


def test_fromarray_palette() -> None:
# Arrange
i = im.convert("L")
Expand Down
2 changes: 2 additions & 0 deletions docs/reference/Image.rst
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ Constructing images
^^^^^^^^^^^^^^^^^^^

.. autofunction:: new
.. autoclass:: SupportsArrayInterface
:show-inheritance:
.. autofunction:: fromarray
.. autofunction:: frombytes
.. autofunction:: frombuffer
Expand Down
21 changes: 17 additions & 4 deletions src/PIL/Image.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
from collections.abc import Callable, MutableMapping
from enum import IntEnum
from types import ModuleType
from typing import IO, TYPE_CHECKING, Any, Literal, cast
from typing import IO, TYPE_CHECKING, Any, Literal, Protocol, cast

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

# VERSION was removed in Pillow 6.0.0.
# PILLOW_VERSION was removed in Pillow 9.0.0.
Expand Down Expand Up @@ -3018,7 +3018,7 @@
return im


def frombuffer(mode, size, data, decoder_name="raw", *args):
def frombuffer(mode, size, data, decoder_name="raw", *args) -> Image:

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

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L3021

Added line #L3021 was not covered by tests
"""
Creates an image memory referencing pixel data in a byte buffer.

Expand Down Expand Up @@ -3074,7 +3074,17 @@
return frombytes(mode, size, data, decoder_name, args)


def fromarray(obj, mode=None):
class SupportsArrayInterface(Protocol):

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

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L3077

Added line #L3077 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Since (I assume) this is not intended to be exported, I would expect we either want this to be private, i.e.

Suggested change
class SupportsArrayInterface(Protocol):
class _SupportsArrayInterface(Protocol):

or in a private module, i.e. PIL._typing.SupportsArrayInterface.

@radarhere what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I was modelling this after our implementation of SupportsGetMesh.

#7812 (comment)

If we're using _SupportsGetMesh as a type hint, this probably is something users might want to use, so should be accessible as a non-underscore thing?

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like the difference is that SupportsGetMesh is a Pillow protocol, whereas SupportsArrayInterface seems to be a numpy protocol. Similarly, we have PIL._typing.SupportsRead for a builtin Python protocol. I would therefore expect users to either use the actual numpy type (e.g. numpy.ndarray) or define their own protocol.

@hugovk Do you have any thoughts as the author of the quoted comment?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, let's go for SupportsArrayInterface, it's probably more useful that way, and it's only a small class. We an always deprecate/remove later if needed.

"""
An object that has an ``__array_interface__`` dictionary.
"""

@property
def __array_interface__(self) -> dict[str, Any]:
raise NotImplementedError()

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

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L3082-L3084

Added lines #L3082 - L3084 were not covered by tests


def fromarray(obj: SupportsArrayInterface, mode: str | None = None) -> Image:

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

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L3087

Added line #L3087 was not covered by tests
"""
Creates an image memory from an object exporting the array interface
(using the buffer protocol)::
Expand Down Expand Up @@ -3153,8 +3163,11 @@
if strides is not None:
if hasattr(obj, "tobytes"):
obj = obj.tobytes()
else:
elif hasattr(obj, "tostring"):

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

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L3166

Added line #L3166 was not covered by tests
obj = obj.tostring()
else:
msg = "'strides' requires either tobytes() or tostring()"
raise ValueError(msg)

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

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L3169-L3170

Added lines #L3169 - L3170 were not covered by tests

return frombuffer(mode, size, obj, "raw", rawmode, 0, 1)

Expand Down
Loading