create set_offsets3d for PathCollection3d #19573
Conversation
timhoffm
left a comment
timhoffm
left a comment
There was a problem hiding this comment.
Sorry to be so critical, but 3D is a mess and without a clear picture where we want to go we risk making it only worse.
We need a bit more thinking to get a consistent 3D API. We currently have,
Text3D.set_positions_3d, having optionalzdirLines3D.set_data_3d , not havingzdir`
Do we want a zdir parameter for data setters?
Also the 3D upcasting is solved differently. Text3D takes zdir but defers the data adaption to draw. In contrast Line3D doesn't even accept zdir in __init__ instead, Axes3D.plot calls line2d_to_3d() which passes zdir to set_3d_properties() and immediately modifies the data. Thus Text3D.get_positions_3d and Line3D.get_data_3d have different interpretations of the returned data, wrt. if zdir is applied.
When and how do we want to apply zdir? What interpretation should the getter return?
|
hmm yeah. Maybe it's best to close this PR and have that worked out on an issue? One other thing to note is that looking at the tutorial the descriptions of
So until now I didn't even really understand what it was for - which I think is evidence for it's currently confused state of API. It's also weird to me that changing the
I think we have different definitions of "so critical" :)
Agreed, I was being irresponsible by jumping the gun to scratch the single itch I had rather than the larger picture. |
This is something I implemented in #18525 when adding the getters/setters on purpose, and I would do the same for the other 3D artists (#18932) except that it requires some work to fix legends. I can't say exactly what the API should be, but we should try to defer as much as possible the conversion to 2D to |
|
Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it. |
github-actions
bot
added
the
status: inactive
3 participants
PR Summary
Add
setandget_offsets3dto the PatchCollection3d object. The casting back to numpy array and transpose in thegetmethod is for consistency with what the 2d version that returns a numpy array with shape (N, 2).Does this need an example or can that be left to #19520?
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).