Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Add type stubs for Axes3D and improve type hints in _AxesBase#30713
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
base:main
Are you sure you want to change the base?
Conversation
QuLogic left a comment
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.
You will need to fix the linting errors. There are also a lot ofAny; I'm not sure how helpful the type hints are with so many of them.
lib/matplotlib/axes/_base.pyi Outdated
| importnumpyasnp | ||
| fromnumpy.typingimportArrayLike | ||
| fromtypingimportAny,Literal,TypeVar,overload | ||
| fromtypingimportAny,Literal,TypeVar,overload,List |
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.
We don't use these deprecated aliases; you should be usinglist.
| def__init__(self,bounds:Sequence[float],transform:Transform)->None: ... | ||
| def__call__(self,ax:Any,renderer:Any)->Bbox: ... |
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.
Don't need the blank lines between methods in type stubs.
| title:Text | ||
| _axis_map:dict[str,Axis] | ||
| _projection_init:Any | ||
| _axis_names:tuple[Literal['x'],Literal['y']]|tuple[Literal['x'],Literal['y'],Literal['z']] |
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.
This type is definitely not literals.
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.
@QuLogic not sure. I think right now this is true at least within the core library. At least PolarAxes still has("x", "y") whether that is desirable is another topic.
lib/mpl_toolkits/mplot3d/axes3d.py Outdated
| fig : Figure | ||
| The parent figure. | ||
| rect : tuple (left, bottom, width, height), default:(0, 0, 1, 1) | ||
| rect : tuple (left, bottom, width, height), default:None |
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.
This change is unrelated, and incorrect.
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.
Please revert. Defaults stated in the docstring are the logical defaults, not necessary what's formally given as the default placeholder object.
lib/mpl_toolkits/mplot3d/axes3d.pyi Outdated
| from __future__importannotations | ||
| __all__= ["Axes3D","_Quaternion","get_test_data"] | ||
| fromtypingimportAny,List,Optional,Sequence,Tuple,Union |
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.
Remove all deprecated aliases. We don't useOptional orUnion either.
lib/mpl_toolkits/mplot3d/axes3d.pyi Outdated
| @@ -0,0 +1,170 @@ | |||
| from __future__importannotations | |||
| __all__= ["Axes3D","_Quaternion","get_test_data"] | |||
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.
Is__all__ actually relevant in type stubs?
ABarpanda commentedNov 1, 2025
Oh thanks |
lib/matplotlib/axes/_axes.py Outdated
| label_namer="y") | ||
| @_docstring.interpd | ||
| defscatter(self,x,y,s=None,c=None,marker=None,cmap=None,norm=None, | ||
| defscatter(self,x,y,z=0,s=None,c=None,marker=None,cmap=None,norm=None, |
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.
You can't change the signature of Axes methods. Please revert.
ABarpanda commentedNov 13, 2025
@timhoffm Please check the latest commit... |
timhoffm left a comment
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.
@ABarpanda please be more carefully to fully implement requested changes and also fix the reported linting / typing errors before requesting a new review.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| title:Text | ||
| _axis_map:dict[str,Axis] | ||
| _projection_init:Any | ||
| _axis_names:tuple[Literal['x'],Literal['y']]|tuple[Literal['x'],Literal['y'],Literal['z']] |
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.
@QuLogic not sure. I think right now this is true at least within the core library. At least PolarAxes still has("x", "y") whether that is desirable is another topic.
lib/mpl_toolkits/mplot3d/axes3d.py Outdated
| fig : Figure | ||
| The parent figure. | ||
| rect : tuple (left, bottom, width, height), default:(0, 0, 1, 1) | ||
| rect : tuple (left, bottom, width, height), default:None |
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.
Please revert. Defaults stated in the docstring are the logical defaults, not necessary what's formally given as the default placeholder object.
timhoffm commentedNov 16, 2025
@ABarpanda you still still have functional changes in a number of places, which also lead to failing tests. I am missing focus and care in this PR. I have marked this as "draft" because we won't spend reviewer time on a sloppy PR. Please go through your PR carefully and fix everything that does not belong in there. When you are done, you are welcome to set the PR to "Ready for review" again. |
Uh oh!
There was an error while loading.Please reload this page.
PR summary
This PR adds a new type stub file for mpl_toolkits.mplot3d.axes3d and makes minor improvements to type hints in matplotlib.axes._base.
Added axes3d.pyi with type definitions for Axes3D, _Quaternion, and get_test_data.
Updated _base.py and _base.pyi with more explicit annotations for _shared_axes and _twinned_axes.
Since there are no dedicated test files for these modules, pytest could not be used.
Type checking was done using mypy; remaining errors are from inherited parent classes, not the new additions.
PR checklist