Skip to content
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

set_aspect for 3D plots #17172

Closed
vondreele opened this issue Apr 17, 2020 · 33 comments
Closed

set_aspect for 3D plots #17172

vondreele opened this issue Apr 17, 2020 · 33 comments
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. topic: mplot3d
Milestone

Comments

@vondreele
Copy link

Bug report

Bug summary

set_aspect does not work for 3D surface plots

Expected outcome

If a sphere is drawn with plot_surface then it should appear as a sphere and not an ellipse that depends on the window sizing. This was achieved in mpl 3.0.3 by calling
set_aspect('equal').
It is now broken in mpl 3.1.1-3.1.3 because the following was inserted in matplotlib/axes/_base.py (lines1279:1282) by the developers
if (not cbook._str_equal(aspect, 'auto')) and self.name == '3d':
raise NotImplementedError(
'It is not currently possible to manually set the aspect '
'on 3D axes')

To get set_aspect to work (correctly, I might add), I have to comment out these lines in my local version of matplotlib. A plotted sphere appears as a sphere independent of the window sizing.

@dstansby
Copy link
Member

See #1077 and #13474 for why this change was made - I'll leave this open as a feature request to add the functionality back, but I think it's a fairly non-trivial feature to implement.

@vondreele
Copy link
Author

vondreele commented Apr 17, 2020 via email

@ImportanceOfBeingErnest
Copy link
Member

Just to make sure we're on the same page here. This comment in #1077 shows that .set_aspect('equal') was just plain wrong.

That is because .set_aspect('equal') was never actually implemented. It just set the aspect in 2D space. In other words, if you ever created a plot with .set_aspect('equal') in the past, that plot would have depicted an incorrect object. A sphere would not ever have been spherical (though it may have looked close enough to not notice).

@vondreele
Copy link
Author

vondreele commented Apr 17, 2020 via email

@QuLogic
Copy link
Member

QuLogic commented Apr 18, 2020

You'll need to post attachments directly; GitHub will not add them from email replies.

@vondreele
Copy link
Author

vondreele commented Apr 18, 2020 via email

@QuLogic
Copy link
Member

QuLogic commented Apr 18, 2020

Directly on GitHub. Inline to the email is still an attachment.

@vondreele
Copy link
Author

Let me try this way:
spheres
The top row is with no set_aspect("equal") the bottom is with a working set_aspect("equal")
This is what should happen.

@ImportanceOfBeingErnest
Copy link
Member

ImportanceOfBeingErnest commented Apr 18, 2020

I see a bit clearer now. The problem is, that while in mpl 3.0 you could (mis)use the 2D-axes' equal aspect to actually achieve a perfect sphere on screen, this is not possible anymore, even in current master with Fix mplot3d projection #8896, or #16472 for that matter, being applied - which should have made the situation better. That is, I was under the impression that #8896 (really #16472) would fix the projection, such that a sphere would always appear circular on screen?

image

image

from itertools import product, combinations
import matplotlib
import matplotlib.pyplot as plt
from mpl_toolkits.mplot3d import Axes3D
import numpy as np


fig = plt.figure()
ax = fig.add_subplot(111, projection='3d')

# Make data
u = np.linspace(0, 2 * np.pi, 100)
v = np.linspace(0, np.pi, 100)
x = np.outer(np.cos(u), np.sin(v))
y = np.outer(np.sin(u), np.sin(v))
z = np.outer(np.ones(np.size(u)), np.cos(v))

# Plot the surface
ax.plot_surface(x, y, z)

#draw cube
r = [-1, 1]
for s, e in combinations(np.array(list(product(r,r,r))), 2):
    if np.sum(np.abs(s-e)) == r[1]-r[0]:
        ax.plot3D(*zip(s,e), color="b")


# Make axes limits 
xyzlim = np.array([ax.get_xlim3d(),ax.get_ylim3d(),ax.get_zlim3d()]).T
XYZlim = [min(xyzlim[0]),max(xyzlim[1])]
ax.set_xlim3d(XYZlim)
ax.set_ylim3d(XYZlim)
ax.set_zlim3d(XYZlim)
try:
    ax.set_aspect('equal')
except NotImplementedError:
    pass

ax.set_title(f"{matplotlib.__version__}")
plt.show()

@vondreele
Copy link
Author

vondreele commented Apr 18, 2020 via email

@ImportanceOfBeingErnest
Copy link
Member

Again, be reminded that we cannot see images attached to emails.

It looks like the case with all axes limits being the same and set_aspect("equal") was the only case that did not produce non-sensical results previously, right? That may be a bit thin for reinstating that otherwise non-working option.

@vondreele
Copy link
Author

vondreele commented Apr 18, 2020 via email

@tacaswell
Copy link
Member

tacaswell commented Apr 22, 2020

ok, so #16472 will be in 3.3 (which is targeted to be out at the end of the month). It looks like that PR a) fixed the bug with apply_aspect that made sure that the 3D axes did not stretch with the figure size (which is the what the top row of #17172 (comment) is showing), however this now pins the aspect ratio of the projected bounding box to be [4, 4, 3] independent of the data limits are which gives the squashing effect that @ImportanceOfBeingErnest . If you manually adjust the z limits to be 3/4 of the x and y limits then you get a spherical looking unit sphere.

from itertools import product, combinations
import matplotlib
import matplotlib.pyplot as plt
from mpl_toolkits.mplot3d import Axes3D
import numpy as np


fig = plt.figure()
ax = fig.add_subplot(111, projection='3d')

# Make data
u = np.linspace(0, 2 * np.pi, 100)
v = np.linspace(0, np.pi, 100)
x = np.outer(np.cos(u), np.sin(v))
y = np.outer(np.sin(u), np.sin(v))
z = np.outer(np.ones(np.size(u)), np.cos(v))

# Plot the surface
ax.plot_surface(x, y, z)

#draw cube
r = [-1, 1]
for s, e in combinations(np.array(list(product(r,r,r))), 2):
    if np.sum(np.abs(s-e)) == r[1]-r[0]:
        ax.plot3D(*zip(s,e), color="b")


# Make axes limits 
xyzlim = np.array([ax.get_xlim3d(),ax.get_ylim3d(),ax.get_zlim3d()]).T
XYZlim = np.asarray([min(xyzlim[0]),max(xyzlim[1])])
ax.set_xlim3d(XYZlim)
ax.set_ylim3d(XYZlim)
ax.set_zlim3d(XYZlim * 3/4)

ax.set_title(f"{matplotlib.__version__}")
plt.show()

Figure_1

I think our options here are:

  • change the hard-coded projection bounding box ratios to be 1:1:1 (not great because still hard-coded, would have to regenerate a whole bunch of images, slight API change (because the 4:4:3 limits were chosen to be close to the old default view), but it may the behavior some fraction of our users expect (judging by the traffic the original issue got))
  • leave the current state of the code and document the 4:4:3 ratio to the hilt. Pro: no code changes or image regeneration, but it is more complexity on users, a bit funny to document, and will make ti more awkward to change in the future
  • add an rcparam to control this and lock in the projection bounding box ratio at axes creation time. Pro: no image re-generation, gives users less fussy control, con: another rcparam
  • add new API to Axes3D to let the user set the projection bounding box to what ever they want. @eric-wieser commented that he had started to work on this and it got messy. This probably should not be laundered through the normal set_aspect due requiring a vector rather than a scalar to describe the aspect ratio. This may be the best long term, but I worry about being able to get the API on this right on a short timeline.
  • something else I have not thought of, but listing bad options has historically been a good way to solicit better options ;)

None of these (except the hypothetical better suggestion) are great, I think we are balancing what we can get done in the next week or so (to hit our 3.3 deadlines) but still ship with some flexibility on this.

My current preference is the rcparam (balances non-fussy user control against quick to do). If we add API in the future, we will probably have to add the rcparam anyway so we are not painting ourselves into a corner.


side note, thank you for the perfect MWE @ImportanceOfBeingErnest :)

@tacaswell tacaswell added this to the v3.3.0 milestone Apr 22, 2020
@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Apr 22, 2020
@tacaswell
Copy link
Member

Labeled as release critical to make sure we a decision about this one way or the other.

@eric-wieser
Copy link
Contributor

I'll check what the state of my branch was - I was at least able to rebase it I think.

@vondreele
Copy link
Author

vondreele commented Apr 22, 2020 via email

@eric-wieser
Copy link
Contributor

My branch is at eric-wieser@set_pb_aspect. I think I never finished rebasing the whole thing, and stalled on #16473.

@tacaswell
Copy link
Member

@vondreele There are two independent aspect ratios here: the data limits and the projection bounding box. If they are equal, a sphere will look like a sphere independent of what the aspect ratios are.

In the 2D case we have the ability to enforce the aspect ratio by adjusting the position / size of the Axes or by adjusting the view limits. It is not immediately clear to me how that generalizes to 3D.

From an API point of view this is hard to manage because we have an over constrained system (the data limits, the projection bounding box aspect ratio, and the ratio between those two). Users want to be able to set any of them independently, but then we have to decide what to do if the users sets inconsistent values.

@vondreele
Copy link
Author

vondreele commented Apr 22, 2020 via email

@ImportanceOfBeingErnest
Copy link
Member

Interesting. I don't immediately see the difference between 2D and 3D. In 2D we have a data aspect and a box aspect.
If the user sets a data aspect, they can choose if they want to have the box change (adjustable="box") or not (adjustable="datalim").
Inversely, if the user sets a box aspect, the box cannot change, so the adjustable will be the data limits.
If the user wants a circle within a square axes, the can
a) set the data aspect to 1, choose adjustable="box", and set equal limits manually, or
b) set the box aspect to 1 and set equal limits manually

I would think that at least in theory this very same concept can apply to 3D axes as well.

@tacaswell
Copy link
Member

@ImportanceOfBeingErnest I would be very happy to be wrong about it generalizing in a direct way:)

@vondreele But I don't think we have a notion of a "sphere sized in screen space" (like we do with markers in the 2D case). I suspect we know enough in the transform stack to set up a meshgrid to behave that way, but I don't know how to do that off the top of my head. Something that is not clear to me is if in the gsas use case you care about the actually spatial extent of the sphere or just the visual appearance and being centered on the correct place in dataspace. A lot of the discussion around this issue has (I think) been centered on making sure that the data-extents are what the user set.

@vondreele
Copy link
Author

Actually the sphere we may draw does have a specific size (not a unit sphere) - see my drawings. Other drawings may be ellipsoids (or other objects) with specific dimensions. The axes scales should match, but the axes all can cover the same range (i.e. 1x1x1 ratio) in our application. In our use the sphere/ellipsoid is centered at the origin; other folks may want them elsewhere.
Question: if the projected bounding box (now hard coded, but if made changeable) were made to match the 3 axes ratios, would any 3D object appear undistorted? I think you implied that above.

@tacaswell
Copy link
Member

tacaswell commented Apr 23, 2020

@vondreele Yes, I think that if you make the aspect ratios of your data limits match the projection bounding box ratios any 3D object should appear undistorted . In the current state with the hard coded bounding box this means your limits must be 4:4:3 to avoid distortion, if we allow the bounding box aspect ratio to be set then you could set it to match your axis limits and get the same thing.

Now that I type that out it seems pretty un-defensible to not provide some sort of API for that.

I probably should have extended the range of x/y rather than shirking z but shrinking z was less typing...

@eric-wieser
Copy link
Contributor

eric-wieser commented Apr 23, 2020

Now that I type that out it seems pretty un-defensible to not provide some sort of API for that.

Agreed - I only omitted it in the original PR because I didn't want to invent API in the same patch as fixing bugs.

Does such an API already exist for 2d plots? The PR I link above adds pb_aspect for 3d plots, but if there's precedent via another route, that approach probably makes little sense.

@vondreele
Copy link
Author

vondreele commented Apr 23, 2020 via email

@ImportanceOfBeingErnest
Copy link
Member

Does such an API already exist for 2d plots? The PR I link above adds pb_aspect for 3d plots

We now have ax.set_box_aspect for 2D axes, which is similar. However, I would recommend naming anything that is specific to 3D axes differently. So maybe .set_3D_aspect and .set_3D_box_aspect or so. This is because a) people will get less confused, and b) the 2D axes methods will probably be needed, so they can't be overridden.

@eric-wieser
Copy link
Contributor

the 2D axes methods will probably be needed, so they can't be overridden.

I'm not convinced, but perhaps the safe bet is:

  1. Use the 3d prefixed versions for the coming release
  2. Overload the 2d versions to raise an exception, until a convincing use case arises - otherwise they might be used in error.
  3. If the use case arises, remove the exceptions in a later release. If it doesn't, alias the 3d ones with the 2d names, again in a future release.

@tacaswell
Copy link
Member

Agreed - I only omitted it in the original PR because I didn't want to invent API in the same patch as fixing bugs.

@eric-wieser That definitely was not meant as a criticism of you, that was poking at myself attempting to defend the status-quo in this thread!

Does such an API already exist for 2d plots?

I would trace back through what apply_aspect does in the 2D case. We have a 2 positions on each axes, an aspect ratio, a setting if we adjust the data limits or view limits, and special casing to make sure that twined axes always are always overlapping. I am not sure I have a clear picture of all of the details of the top of my head (@efiring is the local expert on this I believe).

I think starting with them as their own methods only on Axes3D is a safe place to start as is making sure the non-operative 2D methods are intentionally broken. If we decided we want to make them alias later that is easier than realizing we really needed both all along.

tacaswell added a commit to tacaswell/matplotlib that referenced this issue May 27, 2020
closes matplotlib#17172

Instead of adding a new method, reuse `box_aspect` expecting a 3
vector.

The default value of `None` is slightly special handled to make sure
we hit the old behavior without having to put long floating point
numbers in the source.
@tacaswell
Copy link
Member

@eric-wieser I have become convinced that you are correct and overloading set_aspect is the right way to do this. For now in #17515 I only added the ability to set the box aspect ratio. I know this is less convenient, but I think this will un-block @vondreele .

from itertools import product, combinations
import matplotlib
import matplotlib.pyplot as plt
from mpl_toolkits.mplot3d import Axes3D
import numpy as np
plt.ion()

fig = plt.figure()
ax = fig.add_subplot(111, projection='3d')

# Make data
u = np.linspace(0, 2 * np.pi, 100)
v = np.linspace(0, np.pi, 100)
x = np.outer(np.cos(u), np.sin(v))
y = np.outer(np.sin(u), np.sin(v))
z = np.outer(np.ones(np.size(u)), np.cos(v))

# Plot the surface
ax.plot_surface(x, y, z)

#draw cube
r = [-1, 1]
for s, e in combinations(np.array(list(product(r,r,r))), 2):
    if np.sum(np.abs(s-e)) == r[1]-r[0]:
        ax.plot3D(*zip(s,e), color="b")


# Make axes limits 
xyzlim = np.array([ax.get_xlim3d(),ax.get_ylim3d(),ax.get_zlim3d()]).T
XYZlim = [min(xyzlim[0]),max(xyzlim[1])]
ax.set_xlim3d(XYZlim)
ax.set_ylim3d(XYZlim)
ax.set_zlim3d(XYZlim)
try:
    ax.set_aspect('equal')
except NotImplementedError:
    pass

ax.set_title(f"{matplotlib.__version__}")
ax.set_box_aspect((1, 1, 1))
plt.show()

so

tacaswell added a commit to tacaswell/matplotlib that referenced this issue May 27, 2020
closes matplotlib#17172

Instead of adding a new method, reuse `box_aspect` expecting a 3
vector.

The default value of `None` is slightly special handled to make sure
we hit the old behavior without having to put long floating point
numbers in the source.
@eric-wieser
Copy link
Contributor

How does that compare to the 2d API?

tacaswell added a commit to tacaswell/matplotlib that referenced this issue May 30, 2020
closes matplotlib#17172

Instead of adding a new method, reuse `box_aspect` expecting a 3
vector.

The default value of `None` is slightly special handled to make sure
we hit the old behavior without having to put long floating point
numbers in the source.
@QuLogic QuLogic closed this as completed in 6915f69 Jun 6, 2020
@tacaswell
Copy link
Member

@eric-wieser It ended up being the same, but with a length 3 vector rather than a scalar.

@jhultman
Copy link

I think the documentation for set_box_aspect can be improved. I had difficulty understanding how to implement this part:

To simulate having equal aspect in data space, set the box aspect to match your data range in each dimension.

Fortunately I happened to come across this github issue and I could refer to @tacaswell's example above. I think it is hard for the average user to understand this feature without some additional details.

@jhrmnn
Copy link

jhrmnn commented Apr 30, 2021

This contrived one-liner sets the box aspect ratio from the current axes limits to achieve the "equal" behavior:

ax.set_box_aspect([ub - lb for lb, ub in (getattr(ax, f'get_{a}lim')() for a in 'xyz')])

lawrennd added a commit to lawrennd/GPy that referenced this issue May 18, 2021
Due to this issue: matplotlib/matplotlib#17172 setting aspect equal doesn't work on 3D axes. Removing for the moment.
lawrennd added a commit to SheffieldML/GPy that referenced this issue May 19, 2021
Due to this issue: matplotlib/matplotlib#17172 setting aspect equal doesn't work on 3D axes. Removing for the moment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. topic: mplot3d
Projects
None yet
Development

No branches or pull requests

8 participants