-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Provide axis('equal') for Axes3D (replaces PR #23017) #23409
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
Conversation
Oh, I see now: the aspect ratio will be set for the current view. Makes perfect sense! There appears to be connection between setting the box aspect to
should do the trick. (Unable to test this right now, so I'll let somebody else decide which approach is better.) |
@tfpf I actually quite like the result with your idea there: v_intervals = np.vstack((self.xaxis.get_view_interval(),
self.yaxis.get_view_interval(),
self.zaxis.get_view_interval()))
deltas = np.ptp(v_intervals, axis=1)
self.set_box_aspect(deltas)
# One liner for an existing axes3d `ax`:
# ax.set_box_aspect(np.ptp(np.array([a.get_view_interval() for a in (ax.xaxis, ax.yaxis, ax.zaxis)]), axis=1)) I had another idea for how this could look like too, similar to the original idea except it keeps the current (here the default 4:4:3) box aspect ratio: v_intervals = np.vstack((self.xaxis.get_view_interval(),
self.yaxis.get_view_interval(),
self.zaxis.get_view_interval()))
mean = np.mean(v_intervals, axis=1)
delta = np.max(np.ptp(v_intervals, axis=1))
deltas = delta * self._box_aspect / min(self._box_aspect)
self.set_xlim3d(mean[0] - deltas[0] / 2., mean[0] + deltas[0] / 2.)
self.set_ylim3d(mean[1] - deltas[1] / 2., mean[1] + deltas[1] / 2.)
self.set_zlim3d(mean[2] - deltas[2] / 2., mean[2] + deltas[2] / 2.) If I had to choose one I think I would choose the last, since the user can precede it with a |
So to summarize, here are the three options I see:
I would vote 3 since it encompasses 1 and keeps clear separation between the functions that change the box aspect and the data aspect. Will make the change to 3 unless there are any objections. Perhaps 2 could be |
e1db44e
to
ea277d4
Compare
Edit: I'll just keep them as the author on the squashed commit. |
This is a nice idea. Could use the
which would make it more closely resemble the behaviour of 2D axes. I am not too familiar with this part of the codebase, though. Perhaps somebody else could also provide their suggestions. |
ea277d4
to
60adfbf
Compare
I think matching that |
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.
Looks good. Changing those gallery examples was also a nice touch!
Nice discussion, which probably shows why it wasn't implemented earlier: there are lots of definitions of "equal" here... One thing that struck me, that of course doesn't have to be part of this, is that maybe one would also like to add something like "equalyxy", "equalxz" and "equalyz" to set two out of three axes equal? |
77a068c
to
daf44a6
Compare
@oscargus good idea, wasn't too hard to add! |
face295
to
cc9e7ba
Compare
Whats new for 3D plot equal aspect ratio Code review updates set_aspect('equal') now adopts current box aspect Update test image Update whats new Update whats new test image
Update docstrings
cc9e7ba
to
da0e607
Compare
Huh, gallery updates must have gotten lost in a rebase, added back in |
As discussed in the dev call yesterday, this is now a blocker for #23449. |
11f6625
to
0bb940e
Compare
🎉 I'm very excited this is going in! |
PR Summary
This replaces #23017. As requested in the thread I rebased and added the What's New entry. I think that @patquem will retain the credit for their work in the first commit, please let me know if I did this wrong.
Closes #22570
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).