-
-
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
fromarray: add type hints #7936
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
I think the actual type would be |
It doesn't have to be a |
The other half of the NumPy support, which also isn't typed yet, is here: Lines 686 to 709 in aeeb596
|
I think |
I think you may need to add numpy to the list of dependencies here Lines 69 to 71 in aeeb596
|
Something like from typing import Protocol, Sequence, TypedDict
class ArrayInterfaceDict(TypedDict):
shape: Sequence[int]
typestr: str
class ArrayInterface(Protocol):
__array_interface__: ArrayInterfaceDict would be a more exact definition of what Pillow requires. |
Adding that to |
Otherwise, I've created adamjstewart#1 as a suggestion to add the protocol. |
Added SupportsArrayInterface
Thanks for the help @radarhere! P.S. Love the profile pic @Yay295, excited for season 3 coming out this weekend! |
@@ -3069,7 +3069,15 @@ def frombuffer(mode, size, data, decoder_name="raw", *args): | |||
return frombytes(mode, size, data, decoder_name, args) | |||
|
|||
|
|||
def fromarray(obj, mode=None): | |||
class SupportsArrayInterface(Protocol): |
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.
Since (I assume) this is not intended to be exported, I would expect we either want this to be private, i.e.
class SupportsArrayInterface(Protocol): | |
class _SupportsArrayInterface(Protocol): |
or in a private module, i.e. PIL._typing.SupportsArrayInterface
.
@radarhere what do you think?
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 was modelling this after our implementation of SupportsGetMesh
.
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?
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 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?
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.
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.
NumPy seems to overload the |
Co-authored-by: Ondrej Baranovič <ondreko.tiba@gmail.com>
Anything else needed for this PR? |
There's an unresolved debate, but I expect it will be resolved sooner or later before the next release. |
Thanks! |
Really excited to see type hints in the latest release!
The only feature I'm using that was missing type hints is
fromarray
, so I added them. Not 100% sure ifnumpy.typing.ArrayLike
is the right way to go, but feel free to suggest alternatives.