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: Add overload to specify output of Colormap.__call__ when possible #26504

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

ksunden
Copy link
Member

@ksunden ksunden commented Aug 12, 2023

PR summary

Closes #26501

Does much the same as #26434, but for Colormap.__call__, noting that the scalar->tuple case is a little different to scalar->scalar

PR checklist

Copy link
Contributor

@neutrinoceros neutrinoceros left a comment

Choose a reason for hiding this comment

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

thank you so much for the quick patch ! I confirm this fixes the issue

@overload
def __call__(
self, X: float, alpha: float | None = ..., bytes: bool = ...
) -> tuple[float, float, float, float]: ...
Copy link
Member

Choose a reason for hiding this comment

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

I’m trying to get my head around why the third option below still needs the union. Are there inputs other than X: float, alpha: float that would return a tuple? Or does the checker check the float, float combination against both because floats are array-like and the checker doesn’t stop after finding the precise match? Or something else?

Copy link
Member Author

@ksunden ksunden Aug 12, 2023

Choose a reason for hiding this comment

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

ArrayLike is a fair bit broader than I would actually like in many cases. Numpy (understandably) does not provide all of its building blocks as public apis, though, so narrowing it would essentially mean reimplementing or vendoring a lot of the machinery. But I suspect there are things that do qualify under arraylike that may not qualify as Sequence (particularly things that provide a __array__ method, though probably still inadvisable to make such a thing that isn't considered a sequence)

Mypy does not like when multiple signatures are given that are incompatible, though and float is included in arraylike, so it's output must also be in the return type.

One option would be to make the fallback Any so that users won't have to deal with a union return type but mypy also doesn't complain. It's a bit of a cop out answer, but sometimes best for situations where the return type is known but cannot be differentiated by input types (in this case because we want the catch all arraylike to be sure we didn't miss anything)

The other aspect I didn't fully think about was if other scalars could be passed, though pretty sure it essentially is floats (and int I guess, but type checkers accept int when typed as float)

TLDR it's a conservative catch all that probably isn't going to be selected in most use cases, but needs to be consistent with the other signature options

@QuLogic QuLogic added this to the v3.8.0 milestone Aug 15, 2023
@QuLogic QuLogic merged commit afa625e into matplotlib:main Aug 15, 2023
40 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Aug 15, 2023
QuLogic added a commit that referenced this pull request Aug 15, 2023
…504-on-v3.8.x

Backport PR #26504 on branch v3.8.x (TYP: Add overload to specify output of Colormap.__call__ when possible)
@ksunden ksunden mentioned this pull request Sep 15, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: type-checking errors with mypy + matplotlib 3.8.0rc1
4 participants