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

TYP: Adjust type hint of Norm.__call__ to return masked array #26434

Merged
merged 2 commits into from
Aug 3, 2023

Conversation

ksunden
Copy link
Member

@ksunden ksunden commented Aug 2, 2023

PR summary

xref pydata/xarray#8030

We were perhaps overly permissive in the return type, I'm pretty sure we always return Masked Array, so may as well say so, so that downstream can rely on that (which they already are, to be clear...)

PR checklist

xref pydata/xarray#8030

We were perhaps overly permissive in the return type, I'm pretty sure we always return Masked Array, so may as well say so, so that downstream can rely on that (which they already are, to be clear...)
@QuLogic
Copy link
Member

QuLogic commented Aug 2, 2023

You can also pass a scalar in and get a scalar out; should we overload that?

@ksunden
Copy link
Member Author

ksunden commented Aug 2, 2023

Hmmmm... right you are... that ... complicates things as "scalar" is not a very easy thing to type hint even those from "ArrayLike" are actually insufficient... (and we do want to accept the non-scalar array likes (not just arrays...)

perhaps

    @overload
    def __call__(self, value: np.ndarray, clip: bool | None = ...) -> np.ma.MaskedArray: ...
    @overload
    def __call__(self, value: ArrayLike, clip: bool | None = ...) -> ArrayLike: ...

It's a little funky because the signatures are overlapping (which can be a problem to mypy... at least xarray is having a problem along those lines, but I think we may be saved by the return types being compatible, just one is more restricted...)

This seems to help with xarray without causing us problems.

Lists and other non-ndarray array likes (including scalars, to the extent that scalars are supported by type hints at all) will just not get the type narrowing that the output is a masked array, but those that care can easily do np.asarray or similar to get that type specifically.

@tacaswell tacaswell added this to the v3.8.0 milestone Aug 2, 2023
lib/matplotlib/colors.pyi Outdated Show resolved Hide resolved
@QuLogic QuLogic merged commit 9bba195 into matplotlib:main Aug 3, 2023
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants