Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Implement warning for Text3D's rotation/rotation_mode parameters#30600
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
lib/mpl_toolkits/mplot3d/art3d.py Outdated
| def__init__(self,x=0,y=0,z=0,text='',zdir='z',axlim_clip=False, | ||
| **kwargs): | ||
| ifkwargs.get('rotation',None)isnotNone: |
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.
Can we change to
| ifkwargs.get('rotation',None)isnotNone: | |
| if"rotation"inkwargs: |
? It's not exactly the same in thatText3D(... roation=None) is differently handled. But unless we have internal reasons to accept that. We can warn on that case as well.
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.
Changed.
I didif kwargs.get('rotation', None) is not None: thinking that if a code were to programmatically passNone as an argument, then it would not invoke the warnings.
On the other hand,if "rotation" in kwargs would show the warnings even if those parameters were set toNone.
lib/mpl_toolkits/mplot3d/art3d.py Outdated
| def__init__(self,x=0,y=0,z=0,text='',zdir='z',axlim_clip=False, | ||
| **kwargs): | ||
| ifkwargs.get('rotation',None)isnotNone: | ||
| warnings.showwarning( |
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 may want to use
| warnings.showwarning( | |
| _api.warn_external( |
https://matplotlib.org/stable/api/_api_api.html#matplotlib._api.warn_external
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.
Done
| """ | ||
| importmath | ||
| importwarnings |
scottshambaughSep 26, 2025 • 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.
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 the unused import, then this PR lgtm.
The linked issue should be kept open after this is merged.
PR summary
Currently, the
rotationandrotation_modeparameters are ignored for theText3Dclass.This affects code that accesses the class directly, as well as via the
Axes3D.textfunction.There is no implementation for the true 3D rotation of text in the current codebase.
It only rotates the text about the axis going through the display screen (set via the
zdirparameter).Implementing it is a serious undertaking which can't be achieved in a short matter of time.
Nevertheless, the parameters for it do exist in the public API which confuses users when they don't work as intended.
Therefore, a warning would be beneficial to warn the user about those two parameters being ignored/unimplemented.
This issue provisionallycloses#30563.