FIX: add set_box_aspect, improve tight bounding box for Axes3D + fix bbox_inches support with fixed box_aspect #17515

tacaswell · 2020-05-27T03:38:39Z

PR Summary

To make bbox_inches='tight' work correctly, we cache the positions of
all of the axes, change their aspect to 'auto', adjust the transforms
on the figure, render the output, and then restore the previous
setting of aspect to each of the figures. This prevent the Axes from
trying to adjust their size on draw. This matters because for the
render that is emitted the figure transforms are out of sync with the
figure size.

As part of cleaning fixing the rendering of Axes3D in #8896 / #16472
we started to use apply_aspect to re-size the area the Axes3D takes up
to maintain a fixed ratio between the axes when the aspect is "auto".
However this conflicts with the expectation in tight_bbox.adjust_bbox
as it assumes setting the aspect to "auto" will prevent any axes
resizing.

This commit addresses this by:

exiting the Axes3D.apply_aspect early if aspect is auto
adding a new aspect mode 'auto_pb' which is the default
for Axes3D which maintains the current master branch behavior
of maintaining a fixed ratio between the sizes of the x, y z axis
independent of the data limits.
re implement several functions on Axes3D to make sure they handle
sharez correctly.

closes #16463.

Starting from @ImportanceOfBeingErnest example

import matplotlib.pyplot as plt
from  mpl_toolkits.mplot3d import Axes3D

plt.rcParams["axes.facecolor"] = "yellow"

fig, ax = plt.subplots(dpi=72, figsize=(6,3), subplot_kw={"projection" : "3d"})
print(ax.get_aspect())

plt.savefig("/tmp/test1.png", facecolor="cyan")
plt.savefig("/tmp/test2.png", facecolor="cyan", bbox_inches="tight")
plt.show()

so better! but we still need at least one more commit fix the tight box computation on the 3D axes.

PR Checklist

Has Pytest style unit tests
Code is Flake 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

tacaswell · 2020-05-27T03:41:29Z

attn @eric-wieser .

attn @jklymak I tried the naive thing patch to Axes3D, but is seems to not be sufficient, what am I missing about how we compute bounding boxes?

jklymak · 2020-05-27T05:08:42Z

I dunno, but I guess it has never worked?

matplotlib/lib/mpl_toolkits/mplot3d/axis3d.py

Lines 403 to 406 in 8e569e9

    
           def get_tightbbox(self, renderer): 
        
               # Currently returns None so that Axis.get_tightbbox 
        
               # doesn't return junk info. 
        
               return None

tacaswell · 2020-05-27T05:24:08Z

Ah, that makes sense, I forgot we replaced the axis classes too.

tacaswell · 2020-05-28T03:20:01Z

The appevyor fail is an upload failure:

found artifact 'dist\matplotlib-3.2.1+5180.g75eedddb-cp37-cp37m-win_amd64.whl' matching 'dist\*' path
5144No artifacts found matching 'result_images\*' path
5145Uploading artifacts...
5146Error uploading artifact the storage: The remote name could not be resolved: 'appveyorcidatav2.blob.core.windows.net'

Related, we should definitely document where those wheels are getting published!

doc/users/next_whats_new/2019-03-25-mplot3d-projection.rst

lib/mpl_toolkits/mplot3d/axes3d.py

lib/mpl_toolkits/mplot3d/axis3d.py

lib/mpl_toolkits/tests/test_mplot3d.py

doc/users/next_whats_new/2019-03-25-mplot3d-projection.rst

This only keeps the pb related changes from the original commit.

tacaswell · 2020-05-30T22:36:00Z

This is what the new test looks like (with text) without 390a57a

doc/users/next_whats_new/2019-03-25-mplot3d-projection.rst

lib/matplotlib/tight_bbox.py

QuLogic · 2020-06-03T20:56:16Z

lib/mpl_toolkits/mplot3d/axes3d.py

@@ -99,7 +102,9 @@ def __init__(
            self._shared_z_axes.join(self, sharez)
            self._adjustable = 'datalim'

-        super().__init__(fig, rect, frameon=True, *args, **kwargs)
+        super().__init__(
+            fig, rect, frameon=True, box_aspect=box_aspect, *args, **kwargs


Huh, shouldn't these keyword arguments be after *args? I guess it must still work though.

lib/mpl_toolkits/mplot3d/axes3d.py

lib/mpl_toolkits/mplot3d/axis3d.py

QuLogic · 2020-06-03T21:10:12Z

Would fix #17172 as well?

tacaswell · 2020-06-03T21:25:57Z

Yes, this will fix #17172 as well.

lib/mpl_toolkits/mplot3d/axes3d.py

efiring · 2020-06-03T21:52:50Z

lib/mpl_toolkits/mplot3d/axes3d.py

+            Changes the physical dimensions of the Axes, such that the ratio
+            of the size of the axis in physical units is x:y:z
+
+            The input will be normalized to a unit vector.


I don't see normalization to a unit vector in the code below. And, is any normalization of the 3 numbers specifying ratios relevant to the user?

The magnitude is controlled by zoom. Maybe phrase this as "Only the ratio matters, the magnitude is discarded"?

Maybe replace both sentences with,

"Sets the ratios of the axis lengths in physical units as x:y:z."

And, earlier, just give the default, (4, 4, 3). If you leave it at that, then giving the zoom with its default and minimal explanation will complete the basics. To go farther, you probably need an example, e.g., what might the call look like if you are making a plot of lake bathymetry and you want 1 m in the vertical to match 100 m in the horizontal. I'm assuming that the sort of thing this is for, but I don't fully understand.

Are these ratios showing the length ratios you would see for a cube viewed along each of 3 axes? So if you have (3, 4, 5), then looking down from the top, the y-axis/x-axis length ratio of the rectangular projection would be 4/3, and looking along the y-axis, the z-axis/x-axis ratio would be 5/3, etc.?

lib/mpl_toolkits/mplot3d/axes3d.py

closes matplotlib#17172 Instead of adding a new method, reuse `box_aspect` expecting a 3 vector. The very long float is to keep the tests passing and preserve the behavior currently on master branch.

The way that bbox_inches='tight' is implemented we need to ensure that we do not try to adjust the aspect during the draw (because we have temporarily de-coupled the reported figure size from the transforms which results in the being distorted). Previously we did not have a way to fix the aspect ratio in screen space of the Axes (only the aspect ratio in dataspace) however in 3.3 we gained this ability for both Axes (matplotlib#14917) and Axes3D (matplotlib#8896 / matplotlib#16472). Rather than add an aspect value to `set_aspect` to handle this case, in the tight_bbox code we monkey-patch the `apply_aspect` method with a no-op function and then restore it when we are done. Previously we would set the aspect to "auto" and restore it in the same places. closes matplotlib#16463.

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

tacaswell · 2020-06-04T02:43:59Z

@efiring The docstring clearer?

efiring · 2020-06-04T17:22:29Z

Summary:

Copy the good paragraph from whatsnew to the set_aspect or set_box_aspect docstring.

@tacaswell Thank you, it's getting better. The remaining overall doc problem is that the only place where there is a useful hint as to how a user can get a desired result is the very helpful paragraph in the whatsnew. I suggest copying this into the docstring, maybe as a note, for either set_aspect or set_box_aspect, or both. As it is now, without that, the set_aspect docstring basically says it doesn't do anything (only option is auto, which is not explained), and refers to the set_box_aspect docstring for more info. Now, the set_box_aspect docstring is almost comprehensible but it's pretty hard to be sure one is correctly visualizing what it is doing, especially in relation to the do-nothing set_aspect. So the poor user gets to the bottom of set_box_aspect docstring, hoping for more hints, and sees only a request to refer to the set_aspect docstring for more info--which isn't there. (It's a docstring reference cycle--call in the garbage collector!)

lib/mpl_toolkits/mplot3d/axes3d.py

tacaswell · 2020-06-05T01:51:14Z

I started with the paragraph in the whats new in both docstrings and then edited to make it flow better.

efiring

Good!

QuLogic · 2020-06-05T08:02:39Z

lib/matplotlib/tight_bbox.py

            ax.set_axes_locator(loc)
+            # delete our no-op function which un-hides the
+            # original method
+            del ax.apply_aspect


I'd like to get confirmation that this covers all the cases that #17561 is supposed to now handle, e.g., overridden methods, etc..

I think it now handles everything. If it does not exist, things are super broken so we only have to handle apply_aspect being in the instance dict or not.

We don't have anything in the test suite that exercises the "in instance dict" code path in the test suite.

lib/mpl_toolkits/mplot3d/axes3d.py

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label May 27, 2020

tacaswell added this to the v3.3.0 milestone May 27, 2020

tacaswell force-pushed the fix_3D_tight_aspect branch from 272c6c3 to f18bd14 Compare May 27, 2020 16:28

tacaswell marked this pull request as ready for review May 27, 2020 21:38

tacaswell mentioned this pull request May 27, 2020

set_aspect for 3D plots #17172

Closed

tacaswell force-pushed the fix_3D_tight_aspect branch from fb91f71 to 1e5b11a Compare May 27, 2020 22:16

timhoffm reviewed May 28, 2020

View reviewed changes

doc/users/next_whats_new/2019-03-25-mplot3d-projection.rst Outdated Show resolved Hide resolved

QuLogic reviewed May 30, 2020

View reviewed changes

tacaswell and others added 3 commits May 30, 2020 18:15

MNT: make sure Axes3D methods handle sharez correctly

bc43279

ENH: implement get_tightbbox on Axis3D

b9f9d2e

ENH: Add support for Axes3D.set_pb_aspect

8c2529a

This only keeps the pb related changes from the original commit.

tacaswell force-pushed the fix_3D_tight_aspect branch from e324271 to 390a57a Compare May 30, 2020 22:15

tacaswell changed the title ~~ENH/FIX: make "auto" aspect with Axes3D match Axes better~~ FIX: add set_box_aspect, improve tight bounding box for Axes3D + fix bbox_inches support with fixeh box_aspect May 30, 2020

tacaswell force-pushed the fix_3D_tight_aspect branch from 6a234a9 to d48b87c Compare June 2, 2020 03:43

QuLogic reviewed Jun 3, 2020

View reviewed changes

tacaswell changed the title ~~FIX: add set_box_aspect, improve tight bounding box for Axes3D + fix bbox_inches support with fixeh box_aspect~~ FIX: add set_box_aspect, improve tight bounding box for Axes3D + fix bbox_inches support with fixed box_aspect Jun 3, 2020

efiring reviewed Jun 3, 2020

View reviewed changes

tacaswell and others added 5 commits June 3, 2020 22:17

ENH: use box_aspect to support setting pb_aspect

6915f69

closes matplotlib#17172 Instead of adding a new method, reuse `box_aspect` expecting a 3 vector. The very long float is to keep the tests passing and preserve the behavior currently on master branch.

MNT: suggestions from review

b021ff4

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

DOC: correct typos and markup

8e6e83c

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

DOC: clearify and simplify docstring

7cde8de

tacaswell force-pushed the fix_3D_tight_aspect branch from 7c21906 to 7cde8de Compare June 4, 2020 02:25

tacaswell mentioned this pull request Jun 4, 2020

MNT: handle generic descriptors in _setattr_cm #17561

Closed

2 tasks

efiring reviewed Jun 4, 2020

View reviewed changes

lib/mpl_toolkits/mplot3d/axes3d.py Outdated Show resolved Hide resolved

lib/mpl_toolkits/mplot3d/axes3d.py Outdated Show resolved Hide resolved

lib/mpl_toolkits/mplot3d/axes3d.py Outdated Show resolved Hide resolved

lib/mpl_toolkits/mplot3d/axes3d.py Outdated Show resolved Hide resolved

tacaswell added 2 commits June 4, 2020 21:49

DOC: clarify docs on Axes3D.set_aspect and Axes3D.set_box_aspect

4c0b944

MNT: do minimal input validation in Axes3D.set_box_aspect

eea3d1a

efiring approved these changes Jun 5, 2020

View reviewed changes

QuLogic reviewed Jun 5, 2020

View reviewed changes

tacaswell and others added 2 commits June 5, 2020 15:00

FIX: correct text of error message

910bfe5

Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

MNT: be more careful about handling pre-monkey patched Axes

8ee34a9

QuLogic approved these changes Jun 6, 2020

View reviewed changes

QuLogic merged commit bbb4856 into matplotlib:master Jun 6, 2020

tacaswell deleted the fix_3D_tight_aspect branch June 8, 2020 21:52

rjpower4 mentioned this pull request Dec 13, 2020

Aspect ratio meta-issue JuliaPlots/Plots.jl#2424

Open

7 tasks

sappelhoff mentioned this pull request Jan 12, 2021

3d plots are no longer spherical sappelhoff/eeg_positions#2

Closed

rgommers mentioned this pull request Feb 27, 2021

DOC: add a tutorial about scipy.stats.qmc scipy/scipy#13487

Merged

tacaswell mentioned this pull request Jun 3, 2021

3d plots with aspect='equal' #1077

Closed

liuzhenqi77 mentioned this pull request Jun 17, 2021

Add custom_aspect parameter to plot_point_brain. netneurolab/netneurotools#104

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: add set_box_aspect, improve tight bounding box for Axes3D + fix bbox_inches support with fixed box_aspect #17515

FIX: add set_box_aspect, improve tight bounding box for Axes3D + fix bbox_inches support with fixed box_aspect #17515

tacaswell commented May 27, 2020

tacaswell commented May 27, 2020

jklymak commented May 27, 2020

tacaswell commented May 27, 2020

tacaswell commented May 28, 2020

tacaswell commented May 30, 2020

QuLogic Jun 3, 2020

QuLogic commented Jun 3, 2020

tacaswell commented Jun 3, 2020

efiring Jun 3, 2020

tacaswell Jun 3, 2020

efiring Jun 4, 2020

tacaswell commented Jun 4, 2020

efiring commented Jun 4, 2020

tacaswell commented Jun 5, 2020

efiring left a comment

QuLogic Jun 5, 2020

tacaswell Jun 5, 2020

FIX: add set_box_aspect, improve tight bounding box for Axes3D + fix bbox_inches support with fixed box_aspect #17515

FIX: add set_box_aspect, improve tight bounding box for Axes3D + fix bbox_inches support with fixed box_aspect #17515

Conversation

tacaswell commented May 27, 2020

PR Summary

PR Checklist

tacaswell commented May 27, 2020

jklymak commented May 27, 2020

tacaswell commented May 27, 2020

tacaswell commented May 28, 2020

tacaswell commented May 30, 2020

QuLogic Jun 3, 2020

Choose a reason for hiding this comment

QuLogic commented Jun 3, 2020

tacaswell commented Jun 3, 2020

efiring Jun 3, 2020

Choose a reason for hiding this comment

tacaswell Jun 3, 2020

Choose a reason for hiding this comment

efiring Jun 4, 2020

Choose a reason for hiding this comment

tacaswell commented Jun 4, 2020

efiring commented Jun 4, 2020

tacaswell commented Jun 5, 2020

efiring left a comment

Choose a reason for hiding this comment

QuLogic Jun 5, 2020

Choose a reason for hiding this comment

tacaswell Jun 5, 2020

Choose a reason for hiding this comment