Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
added typing_extensions.Self to _AxesBase.twinx#28625
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join uson gitter for real-time discussion.
For details on testing, writing docs, and our review process, please seethe developer guide
We strive to be a welcoming and open project. Please follow ourCode of Conduct.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Additionally, we are now python 3.10+ on main anyway, so not extensions is not actually needed for Self. Note however that to be backported to the next micro release, we would need to take care there, since it still runs on py 3.9. |
|
ksunden commentedJul 31, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Ah, I remembered Self coming in (at least added in some places) with recent changes (#28503) that were broadly related to py 3.10 being the new minimum. Regardless, relying on typing_extensions in I do not think we want to make it aruntime dependency necessarily. All that said, in this instance, I don't actually think For twinx/twiny, the Axes that is returned is created by This means that if you call Thus the actual correct type hint is Created here: matplotlib/lib/matplotlib/axes/_base.py Lines 4522 to 4525 in44be14c
Type hints for those methods: matplotlib/lib/matplotlib/figure.pyi Lines 70 to 93 in44be14c
This generally falls into the category of However... I think |
It's generally not clear what twinning should mean on arbitrary axes or that it's useful. For example, where should the second radial axis on polar be placed? Polar does not even have a second radial axis. While it would be technically feasible for 3d and geo axis, the resulting plot is likely very difficult to interpret. For now, I'd pragmatically say that we just don't provide proper twinning for these niche cases and set the return type to |
story645 commentedAug 1, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Um this has me realizing that a twinx geoAxis that shows the 0-360 convention and the 180W-180E convention could be useful for teaching both 😅 But also that's functionality that probably could live in cartopy if anyone wanted it. |
7abb3ba
intomatplotlib:mainUh oh!
There was an error while loading.Please reload this page.
Thanks@randolf-scholz! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again. |
…625-on-v3.9.xBackport PR#28625 on branch v3.9.x (added typing_extensions.Self to _AxesBase.twinx)
PR summary
typing_extensions >= 4.0.0
as a dependency_AxesBase.twinx
and_AxesBase.twiny
with return typetyping_extensions.Self
_AxesBase.twinx()
#28624