Merge pull request matplotlib#11146 from Zac-HD/refactor-margins

dstansby · web-flow · commit bc2ea01635d8 · 2018-05-02T14:15:34.000+01:00
Explicit args and refactor Axes.margins
diff --git a/doc/api/next_api_changes/2018-04-29-ZHD.rst b/doc/api/next_api_changes/2018-04-29-ZHD.rst
@@ -0,0 +1,13 @@
+Improved call signature for Axes.margins()
+------------------------------------------
+
+:meth:`matplotlib.axes.Axes.margins` and :meth:`mpl_toolkits.mplot3d.Axes3D.margins`
+no longer accept arbitrary keywords.  ``TypeError`` will therefore be raised
+if unknown kwargs are passed; previously they would be silently ignored.
+
+If too many positional arguments are passed, ``TypeError`` will be raised
+instead of ``ValueError``, for consistency with other call-signature violations.
+
+``Axes3D.margins`` now raises ``TypeError`` instead of emitting a deprecation
+warning if only two positional arguments are passed.  To supply only ``x`` and
+``y`` margins, use keyword arguments.
diff --git a/lib/matplotlib/axes/_base.py b/lib/matplotlib/axes/_base.py
@@ -2229,7 +2229,7 @@ def set_ymargin(self, m):
         self._ymargin = m
         self.stale = True
 
-    def margins(self, *args, **kw):
+    def margins(self, *margins, x=None, y=None, tight=True):
         """
         Set or retrieve autoscaling margins.
 
@@ -2250,10 +2250,11 @@ def margins(self, *args, **kw):
             margins(..., tight=False)
 
         All three forms above set the xmargin and ymargin parameters.
-        All keyword parameters are optional.  A single argument
+        All keyword parameters are optional.  A single positional argument
         specifies both xmargin and ymargin. The padding added to the end of
         each interval is *margin* times the data interval. The *margin* must
-        be a float in the range [0, 1].
+        be a float in the range [0, 1].  Passing both positional and keyword
+        arguments for xmargin and/or ymargin is invalid.
 
         The *tight* parameter is passed to :meth:`autoscale_view`
         , which is executed after a margin is changed; the default here is
@@ -2267,27 +2268,30 @@ def margins(self, *args, **kw):
         it is used in autoscaling.
 
         """
-        if not args and not kw:
+        if margins and x is not None and y is not None:
+            raise TypeError('Cannot pass both positional and keyword '
+                            'arguments for x and/or y.')
+        elif len(margins) == 1:
+            x = y = margins[0]
+        elif len(margins) == 2:
+            x, y = margins
+        elif margins:
+            raise TypeError('Must pass a single positional argument for all '
+                            'margins, or one for each margin (x, y).')
+
+        if x is None and y is None:
+            if tight is not True:
+                warnings.warn('ignoring tight=%r in get mode' % (tight,))
             return self._xmargin, self._ymargin
 
-        tight = kw.pop('tight', True)
-        mx = kw.pop('x', None)
-        my = kw.pop('y', None)
-        if len(args) == 1:
-            mx = my = args[0]
-        elif len(args) == 2:
-            mx, my = args
-        elif len(args) > 2:
-            raise ValueError("more than two arguments were supplied")
-        if mx is not None:
-            self.set_xmargin(mx)
-        if my is not None:
-            self.set_ymargin(my)
-
-        scalex = (mx is not None)
-        scaley = (my is not None)
+        if x is not None:
+            self.set_xmargin(x)
+        if y is not None:
+            self.set_ymargin(y)
 
-        self.autoscale_view(tight=tight, scalex=scalex, scaley=scaley)
+        self.autoscale_view(
+            tight=tight, scalex=(x is not None), scaley=(y is not None)
+        )
 
     def set_rasterization_zorder(self, z):
         """
diff --git a/lib/mpl_toolkits/mplot3d/axes3d.py b/lib/mpl_toolkits/mplot3d/axes3d.py
@@ -384,7 +384,7 @@ def set_zmargin(self, m):
         self._zmargin = m
         self.stale = True
 
-    def margins(self, *args, **kw):
+    def margins(self, *margins, x=None, y=None, z=None, tight=True):
         """
         Convenience method to set or retrieve autoscaling margins.
 
@@ -404,8 +404,12 @@ def margins(self, *args, **kw):
             margins(..., tight=False)
 
         All forms above set the xmargin, ymargin and zmargin
-        parameters. All keyword parameters are optional.  A single argument
-        specifies xmargin, ymargin and zmargin.  The *tight* parameter
+        parameters. All keyword parameters are optional.  A single
+        positional argument specifies xmargin, ymargin and zmargin.
+        Passing both positional and keyword arguments for xmargin,
+        ymargin, and/or zmargin is invalid.
+
+        The *tight* parameter
         is passed to :meth:`autoscale_view`, which is executed after
         a margin is changed; the default here is *True*, on the
         assumption that when margins are specified, no additional
@@ -420,41 +424,33 @@ def margins(self, *args, **kw):
         .. versionadded :: 1.1.0
             This function was added, but not tested. Please report any bugs.
         """
-        if not args and not kw:
+        if margins and x is not None and y is not None and z is not None:
+            raise TypeError('Cannot pass both positional and keyword '
+                            'arguments for x, y, and/or z.')
+        elif len(margins) == 1:
+            x = y = z = margins[0]
+        elif len(margins) == 3:
+            x, y, z = margins
+        elif margins:
+            raise TypeError('Must pass a single positional argument for all '
+                            'margins, or one for each margin (x, y, z).')
+
+        if x is None and y is None and z is None:
+            if tight is not True:
+                warnings.warn('ignoring tight=%r in get mode' % (tight,))
             return self._xmargin, self._ymargin, self._zmargin
 
-        tight = kw.pop('tight', True)
-        mx = kw.pop('x', None)
-        my = kw.pop('y', None)
-        mz = kw.pop('z', None)
-        if not args:
-            pass
-        elif len(args) == 1:
-            mx = my = mz = args[0]
-        elif len(args) == 2:
-            warnings.warn(
-                "Passing exactly two positional arguments to Axes3D.margins "
-                "is deprecated.  If needed, pass them as keyword arguments "
-                "instead", cbook.mplDeprecation)
-            mx, my = args
-        elif len(args) == 3:
-            mx, my, mz = args
-        else:
-            raise ValueError(
-                "Axes3D.margins takes at most three positional arguments")
-        if mx is not None:
-            self.set_xmargin(mx)
-        if my is not None:
-            self.set_ymargin(my)
-        if mz is not None:
-            self.set_zmargin(mz)
-
-        scalex = mx is not None
-        scaley = my is not None
-        scalez = mz is not None
+        if x is not None:
+            self.set_xmargin(x)
+        if y is not None:
+            self.set_ymargin(y)
+        if z is not None:
+            self.set_zmargin(z)
 
-        self.autoscale_view(tight=tight, scalex=scalex, scaley=scaley,
-                                         scalez=scalez)
+        self.autoscale_view(
+            tight=tight, scalex=(x is not None), scaley=(y is not None),
+            scalez=(z is not None)
+        )
 
     def autoscale(self, enable=True, axis='both', tight=None):
         """
