Fix incorrect transform in apply_aspect.

anntzer · anntzer · commit 59a6a2fdfc16 · 2019-07-28T23:44:19.000+02:00
x_trf goes from rawdata-space to scaled-space, so it's what should get applied to datalims, not x_trf.inverted(). So ``` x0, x1 = map(x_trf.inverted().transform, dL.intervalx) y0, y1 = map(y_trf.inverted().transform, dL.intervaly) ``` from 87c742b should have been ``` x0, x1 = map(x_trf.transform, dL.intervalx) y0, y1 = map(y_trf.transform, dL.intervaly) ``` However, fixing that triggered a failure for test_aspect_nonlinear_adjustable_datalim which had been added in that commit, and fixing that unraveled more issues. The basic question is, when aspect is set and adjustable="datalim", should we change the x limits or the y limits to get the correct aspect? The old code used some complex conditions, which I actually haven't managed to fully understand, to either expand or shrink one of the axises. Instead, just choose to always expand (rather than shrink) one of the axises, which will avoid sending artists out-of-bounds. (The sole exception is in care of shared axes, which we do not touch as explained in the comment.) This patch caused a change in the autolimiting of test_axes.py::test_pie_frame_grid which was buggy anyways, I forced the old behavior by setting x/ylims manually (after checking that the default is to expand the limits).
diff --git a/lib/matplotlib/axes/_base.py b/lib/matplotlib/axes/_base.py
@@ -1501,55 +1501,42 @@ def apply_aspect(self, position=None):
         box_aspect = fig_aspect * (h / w)
         data_ratio = box_aspect / aspect
 
-        y_expander = data_ratio * xsize / ysize - 1
-        # If y_expander > 0, the dy/dx viewLim ratio needs to increase
-        if abs(y_expander) < 0.005:
+        y_expander = data_ratio * xsize / ysize
+        # If y_expander > 1, the dy/dx viewLim ratio needs to increase
+        if abs(y_expander - 1) < 0.005:
             return
 
-        dL = self.dataLim
-        x0, x1 = map(x_trf.inverted().transform, dL.intervalx)
-        y0, y1 = map(y_trf.inverted().transform, dL.intervaly)
-        xr = 1.05 * (x1 - x0)
-        yr = 1.05 * (y1 - y0)
-
-        xmarg = xsize - xr
-        ymarg = ysize - yr
-        Ysize = data_ratio * xsize
-        Xsize = ysize / data_ratio
-        Xmarg = Xsize - xr
-        Ymarg = Ysize - yr
-        # Setting these targets to, e.g., 0.05*xr does not seem to help.
-        xm = 0
-        ym = 0
-
+        # What axes do we adjust?
+        # - We can't adjust shared axes because it's too easy to break sharing
+        #   given the sequential handling of axes (that may be possible if we
+        #   first collected all the constraints on all axes and considered them
+        #   all at once).
+        # - Assuming no constraints from sharing, prefer expanding axes rather
+        #   than shrinking them, as the latter can hide plot elements.
         shared_x = self in self._shared_x_axes
         shared_y = self in self._shared_y_axes
-        # Not sure whether we need this check:
         if shared_x and shared_y:
             raise RuntimeError("adjustable='datalim' is not allowed when both "
                                "axes are shared")
-
-        # If y is shared, then we are only allowed to change x, etc.
-        if shared_y:
-            adjust_y = False
-        else:
-            if xmarg > xm and ymarg > ym:
-                adjy = ((Ymarg > 0 and y_expander < 0) or
-                        (Xmarg < 0 and y_expander > 0))
-            else:
-                adjy = y_expander > 0
-            adjust_y = shared_x or adjy  # (Ymarg > xmarg)
-
-        if adjust_y:
-            yc = 0.5 * (ymin + ymax)
-            y0 = yc - Ysize / 2.0
-            y1 = yc + Ysize / 2.0
-            self.set_ybound(*map(y_trf.inverted().transform, (y0, y1)))
+        if shared_x:
+            adjust = "y"
+        elif shared_y:
+            adjust = "x"
+        elif y_expander > 1:
+            adjust = "y"
+        else:  # y_expander < 1
+            adjust = "x"
+
+        if adjust == "y":
+            yc = (ymin + ymax) / 2
+            ymin = yc - (yc - ymin) * y_expander
+            ymax = yc + (ymax - yc) * y_expander
+            self.set_ybound(*map(y_trf.inverted().transform, (ymin, ymax)))
         else:
-            xc = 0.5 * (xmin + xmax)
-            x0 = xc - Xsize / 2.0
-            x1 = xc + Xsize / 2.0
-            self.set_xbound(*map(x_trf.inverted().transform, (x0, x1)))
+            xc = (xmin + xmax) / 2
+            xmin = xc - (xc - xmin) / y_expander
+            xmax = xc + (xmax - xc) / y_expander
+            self.set_xbound(*map(x_trf.inverted().transform, (xmin, xmax)))
 
     def axis(self, *args, emit=True, **kwargs):
         """
diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py
@@ -4928,6 +4928,13 @@ def test_pie_frame_grid():
             frame=True, center=(3, 5))
     # Set aspect ratio to be equal so that pie is drawn as a circle.
     plt.axis('equal')
+    plt.gcf().canvas.draw()
+    xmin, xmax = plt.xlim()
+    ymin, ymax = plt.ylim()
+    assert xmin < 0 and xmax > 7 and ymin == 0 and ymax == 7
+    # These limits reproduce an old, buggy implementation of axis("equal").
+    plt.xlim(0, 7)
+    plt.ylim(0.7903225806451615, 6.209677419354838)
 
 
 @image_comparison(['pie_rotatelabels_true.png'])
