mplot3d: make Axes3D.annotate accept (x, y, z) and reproject on draw#31267
mplot3d: make Axes3D.annotate accept (x, y, z) and reproject on draw#31267sanrishi wants to merge 2 commits intomatplotlib:mainfrom
Conversation
scottshambaugh
left a comment
There was a problem hiding this comment.
I tried this out locally and it works well! A number of comments here, but I agree on the overall direction.
Needed docs updates:
- A What's New entry
- A new gallery example for annotations in 3D (somewhat like https://matplotlib.org/devdocs/gallery/mplot3d/text3d.html, though I think this example is messy)
| ``'data'``, the annotated position *xy* may be a 3-tuple *(x, y, z)* in | ||
| 3D data coordinates; in that case, the annotation will reproject during | ||
| draws (e.g. on rotation / zoom). | ||
| """ |
There was a problem hiding this comment.
This docstring needs to be filled out. Describe the difference in behavior between len 2 and len 3 inputs for xy and xytext, and add detailed Parameters descriptions like the other functions here do.
| text3D = text | ||
| text2D = Axes.text | ||
|
|
||
| def annotate(self, text, xy, xytext=None, xycoords='data', textcoords=None, |
There was a problem hiding this comment.
I think keeping these as "xy" args rather than "xyz" args is the right move. It means 3D has the same API as 2D, but with the special behavior of being able to pass in 3D coordinates.
There is the option of renaming these and making the "xy" args keyword only with a deprecation notice, but I don't think that's worth it.
| assert clip_box is not None | ||
| assert np.all(clip_box.extents == ax.bbox.extents) | ||
|
|
||
|
|
There was a problem hiding this comment.
We will need at least:
- Image comparison tests that compare the 2D xytext cases here (both 2D and 3D point positions) with a direct call of
Axes.annotate. This ensures equivalency with the new method. - Image comparison tests for axlim_clip cases.
- A new baseline image test with different cases. One of the axes should be log scale.
- Take a look at the tests we have for annotate in the 2D case to see what else might be needed
| arrowprops=arrowprops, annotation_clip=annotation_clip, **kwargs) | ||
|
|
||
| if xyz is not None: | ||
| art3d.annotation_2d_to_3d(a, xyz, xyztext=xyztext, axlim_clip=axlim_clip) |
There was a problem hiding this comment.
Probably want to issue a warning for the all-2D case if axlim_clip is supplied, since it's not used.
| arrowprops=arrowprops, annotation_clip=annotation_clip, **kwargs) | ||
|
|
||
| if xyz is not None: | ||
| art3d.annotation_2d_to_3d(a, xyz, xyztext=xyztext, axlim_clip=axlim_clip) |
There was a problem hiding this comment.
When axlim_clip = True, I see the following behavior:
- xyz coordinate is clipped, the text and arrow disappear (I think this is correct)
- The text coordinate is clipped, matplotlib crashes (needs to be fixed)
| text, xy, xytext=xytext, xycoords=xycoords, textcoords=textcoords, | ||
| arrowprops=arrowprops, annotation_clip=annotation_clip, **kwargs) | ||
|
|
||
| if xyz is not None: |
There was a problem hiding this comment.
There should also be a path that allows xyztext is not None and xyz is None (2D data location, 3D text location).
| return None | ||
|
|
||
|
|
||
| class Annotation3D(mtext.Annotation): |
There was a problem hiding this comment.
Annotation inherits from Text. How much effort would it be to also inherit from Text3D so we get zdir handling, etc? I think not strictly necessary since we can add later.
PR summary
AI Disclosure
I used an AI coding assistant to help navigate the Matplotlib codebase (specifically to quickly locate the clipping logic inside text.py and art3d.py) and to help draft the initial Python code for the Annotation3D class based on the architectural plan approved in Issue #7927.
All AI-generated code was heavily reviewed, manually debugged, and verified by me. I personally ran the pytest suite locally and performed manual visual testing with 3D rotations to ensure the math and clipping behaviors perfectly matched standard 2D Matplotlib semantics.
Summary
Fixes mplot3d annotations being anchored in projected 2D snapshot space instead of 3D data space (#7927). Today Axes3D inherits the 2D Axes.annotate, so annotations do not reproject when the 3D view changes (rotate/zoom), even though 3D artists update using Axes3D.M.
What this PR changes
Text3D pattern).
projection).
Clipping / layout behavior
Visual Proof & Minimal Reproducible Example
Here is an MRE demonstrating the fix. The black arrow uses the new 3D tracking, while the orange arrow shows the old 2D snapshot drifting bug.
Screenshot
PR checklist