-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[TYP] Fix overload of pyplot.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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Interesting,
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). |
Let's see if we can keep the |
I think we may be able to get by by simply changing only the "fallback" case from having a union return type to just being The truth is that the type hint is not 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 wanting |
Thanks, I think this is a better solution than what I came up with.
This is where I disagree. It is true that the return type can be
This is referred to as dynamic typing. In Python Any 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 |
Is anyone able to review this? Would love to get this into 3.9.2. |
…518-on-v3.9.x Backport PR #28518 on branch v3.9.x ([TYP] Fix overload of `pyplot.subplots`)
I can confirm that this change fixes mypy error in my job's codebase. |
PR summary
Fixes #27001 (comment)
My solution is to have 3 possible cases:
squeeze=True
,nrows=1
,ncols=1
:Axes
squeeze=True
:np.ndarray
squeeze=False
:np.ndarray
1 and 3 already existed. 2 is hopefully treated as a fallback by mypy for when either
nrows
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