-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Implement arrow3d method in Axes3D
#30517
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
base: main
Are you sure you want to change the base?
Conversation
34397ac to
893c34e
Compare
story645
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.
Thanks for tackling this! I would love to see a version of this in the library, but my strong preference is for the signature to match FancyArrowPatch.
lib/mpl_toolkits/mplot3d/art3d.py
Outdated
| """ | ||
| 3D FancyArrowPatch object. | ||
| """ | ||
| def __init__(self, xs, ys, zs, *args, **kwargs): |
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.
| def __init__(self, xs, ys, zs, *args, **kwargs): | |
| def __init__(self, posA, posB, *args, **kwargs): |
I think the signature should match the FancyArrowPatch signature unless you have a reason not to. I implemented a rudimentary version with this signature:
https://github.com/story645/team/blob/00c7374cefb216e51b8f8350a4b5512ec1068a99/paper/figcode/nontrivial.py#L54-L64
|
IMO the major challenge here is semantics, scope and API design and that will entail the same topics as #29826 (comment), which lead to stalling in that thread. After all, we should have a consistent story between 2d and 3d. - I suspect to move forward here, we have to pick up the discussion from #29826. |
893c34e to
ba9dcd7
Compare
|
While I think the more substantive discussion/change is likely the API discussion above/in the issue linked above, a small technical note that the test failure on ARM (at least, logs are gone for the azure) does appear to be a small deviation in the image, most likely due to rounding modes on the CPU architecture (RMS 0.001) so adding a tolerance may be necessary. |
|
This PR is similar to Vector #22435, and is intended to plot 'data' rather than 'annotate'. However the Vector PR is currently inactive. It seems like this is because of the presence of another function If there are plans for Vector to be merged, I could follow the same API (mentioned in this comment) for this PR and also rename the function from I would like to know your thoughts on these @timhoffm |
|
@AdwaithBatchu as written in #30517 (comment) I think we have to first solve the 2D case to ensure API consistency. We currently do not have the resources and priority to work on that. This unfortunately means working on arrow3d is blocked. As a temporary solution, you could create an example in the 3D section of our gallery, containing |
PR summary
closes #22571
This PR implements a method to plot a 3D arrow.
The
ax.arrow3dmethod plots a single arrow between specified end, start points. It returns an instance of theArrow3Dclass, which inherits from theFancyArrowPatchclass. Additional kwargs are passed on toFancyArrowPatchto change properties of the arrow.PR checklist