Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[TYP] Fix overload ofpyplot.subplots#28518

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
timhoffm merged 5 commits intomatplotlib:mainfromadamjstewart:types/subplots
Jul 19, 2024

Conversation

adamjstewart
Copy link
Contributor

PR summary

Fixes#27001 (comment)

My solution is to have 3 possible cases:

  1. squeeze=True,nrows=1,ncols=1:Axes
  2. squeeze=True:np.ndarray
  3. squeeze=False:np.ndarray

1 and 3 already existed. 2 ishopefully treated as a fallback by mypy for when eithernrows orncols is not 1. All of my testing withreveal_type works as expected, but I would love suggestions for how to formally test this in CI so it doesn't happen again.

PR checklist

@adamjstewart
Copy link
ContributorAuthor

Interesting,mypy is fine with this approach butstubtest is not:

error: not checking stubs due to mypy build errors:lib/matplotlib/figure.pyi:95: error: Overloaded function signatures 1 and 2 overlap with incompatible return types  [misc]lib/matplotlib/pyplot.py:1566: error: Overloaded function signatures 1 and 2 overlap with incompatible return types  [misc]

Is it possible to say all integers except 1? If not, we may have to go back to partially dynamic typing as suggested in#27001 (comment).

@adamjstewart
Copy link
ContributorAuthor

Let's see if we can keep the.py code specific and only loosen the restrictions on the.pyi stubs.

@ksunden
Copy link
Member

I think we may be able to get by by simply changingonly the "fallback" case from having a union return type to just beingAny, but keeping the more specific versions to provide the benefits of having type hints when we can.

The truth is that the type hint isnot wrong... it is, to the best of our ability within the type system, giving an accurate description of the return type, which we cannot say whether it will be a single Axes or an ndarray based on the type...

I specifically called out potentially wantingAny for this case to avoid union types on returns in#27001 (review)

adamjstewart reacted with thumbs up emoji

@adamjstewart
Copy link
ContributorAuthor

adamjstewart commentedJul 11, 2024
edited
Loading

I think we may be able to get by by simply changing only the "fallback" case from having a union return type to just beingAny, but keeping the more specific versions to provide the benefits of having type hints when we can.

Thanks, I think this is a better solution than what I came up with.

The truth is that the type hint isnot wrong...

This is where I disagree. It is true that the return type can beAxes ornp.ndarray, but it is not correct to say that the return type isAxes | np.ndarray.

we cannot say whether it will be a single Axes or an ndarray based on the type...

This is referred to asdynamic typing. In PythonAny is used "to indicate that a value is dynamically typed." Using a union as a return type implies that the return value can be of either type,and that all uses of that function must be able to handle both. Unions are almost never used in return types, only in parameter types.12

Footnotes

  1. https://github.com/python/mypy/issues/1693

  2. https://github.com/python/typing/issues/566

@adamjstewart
Copy link
ContributorAuthor

Is anyone able to review this? Would love to get this into 3.9.2.

@timhoffmtimhoffm added this to thev3.9.2 milestoneJul 19, 2024
@timhoffmtimhoffm merged commit087a142 intomatplotlib:mainJul 19, 2024
44 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull requestJul 19, 2024
QuLogic added a commit that referenced this pull requestJul 20, 2024
…518-on-v3.9.xBackport PR#28518 on branch v3.9.x ([TYP] Fix overload of `pyplot.subplots`)
@kalvdans
Copy link

I can confirm that this change fixes mypy error in my job's codebase.

adamjstewart reacted with hooray emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ksundenksundenksunden approved these changes

@timhoffmtimhoffmtimhoffm approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.9.2
Development

Successfully merging this pull request may close these issues.

4 participants
@adamjstewart@ksunden@kalvdans@timhoffm

[8]ページ先頭

©2009-2025 Movatter.jp