Move integerness checks to SubplotSpec._from_subplot_args.

anntzer · anntzer · commit 3ae847d4fe26 · 2020-05-07T18:41:51.000+02:00
This avoids having to later do them same for other places (axes_grid)
which rely on the same machinery to convert three-digit specifications.

I'll assume that the already existing deprecation note for add_subplot
is effectively sufficient.
diff --git a/lib/matplotlib/figure.py b/lib/matplotlib/figure.py
@@ -1264,13 +1264,16 @@ def add_subplot(self, *args, **kwargs):
 
         Parameters
         ----------
-        *args, int or (int, int, int) or `SubplotSpec`, default: (1, 1, 1)
+        *args, int, (int, int, *index*), or `SubplotSpec`, default: (1, 1, 1)
             The position of the subplot described by one of
 
             - Three integers (*nrows*, *ncols*, *index*). The subplot will
               take the *index* position on a grid with *nrows* rows and
               *ncols* columns. *index* starts at 1 in the upper left corner
-              and increases to the right.
+              and increases to the right.  *index* can also be a two-tuple
+              specifying the (*first*, *last*) indices (1-based, and including
+              *last*) of the subplot, e.g., ``fig.add_subplot(3, 1, (1, 2))``
+              makes a subplot that spans the upper 2/3 of the figure.
             - A 3-digit integer. The digits are interpreted as if given
               separately as three single-digit integers, i.e.
               ``fig.add_subplot(235)`` is the same as
@@ -1362,48 +1365,21 @@ def add_subplot(self, *args, **kwargs):
             raise TypeError(
                 "add_subplot() got an unexpected keyword argument 'figure'")
 
-        nargs = len(args)
-        if nargs == 0:
-            args = (1, 1, 1)
-        elif nargs == 1:
-            if isinstance(args[0], Integral):
-                if not 100 <= args[0] <= 999:
-                    raise ValueError(f"Integer subplot specification must be "
-                                     f"a three-digit number, not {args[0]}")
-                args = tuple(map(int, str(args[0])))
-            elif isinstance(args[0], (SubplotBase, SubplotSpec)):
-                pass  # no further validation or normalization needed
-            else:
-                raise TypeError('Positional arguments are not a valid '
-                                'position specification.')
-        elif nargs == 3:
-            for arg in args:
-                if not isinstance(arg, Integral):
-                    cbook.warn_deprecated(
-                        "3.3",
-                        message="Passing non-integers as three-element "
-                                "position specification is deprecated since "
-                                "%(since)s and will be removed %(removal)s.")
-            args = tuple(map(int, args))
-        else:
-            raise TypeError(f'add_subplot() takes 1 or 3 positional arguments '
-                            f'but {nargs} were given')
-
-        if isinstance(args[0], SubplotBase):
+        if len(args) == 1 and isinstance(args[0], SubplotBase):
             ax = args[0]
             if ax.get_figure() is not self:
                 raise ValueError("The Subplot must have been created in "
                                  "the present figure")
             # make a key for the subplot (which includes the axes object id
             # in the hash)
             key = self._make_key(*args, **kwargs)
+
         else:
+            if not args:
+                args = (1, 1, 1)
             projection_class, kwargs, key = \
                 self._process_projection_requirements(*args, **kwargs)
-
-            # try to find the axes with this key in the stack
-            ax = self._axstack.get(key)
-
+            ax = self._axstack.get(key)  # search axes with this key in stack
             if ax is not None:
                 if isinstance(ax, projection_class):
                     # the axes already existed, so set it as active & return
@@ -1416,7 +1392,6 @@ def add_subplot(self, *args, **kwargs):
                     # Without this, add_subplot would be simpler and
                     # more similar to add_axes.
                     self._axstack.remove(ax)
-
             ax = subplot_class_factory(projection_class)(self, *args, **kwargs)
 
         return self._add_axes_internal(key, ax)
diff --git a/lib/matplotlib/gridspec.py b/lib/matplotlib/gridspec.py
@@ -44,6 +44,10 @@ def __init__(self, nrows, ncols, height_ratios=None, width_ratios=None):
             relative height of ``height_ratios[i] / sum(height_ratios)``.
             If not given, all rows will have the same height.
         """
+        if not isinstance(nrows, Integral) or nrows <= 0:
+            raise ValueError(f"Number of rows must be > 0, not {nrows}")
+        if not isinstance(ncols, Integral) or ncols <= 0:
+            raise ValueError(f"Number of columns must be > 0, not {ncols}")
         self._nrows, self._ncols = nrows, ncols
         self.set_height_ratios(height_ratios)
         self.set_width_ratios(width_ratios)
@@ -642,36 +646,46 @@ def _from_subplot_args(figure, args):
         - a `.SubplotSpec` -- returned as is;
         - one or three numbers -- a MATLAB-style subplot specifier.
         """
+        message = ("Passing non-integers as three-element position "
+                   "specification is deprecated since %(since)s and will be "
+                   "removed %(removal)s.")
         if len(args) == 1:
             arg, = args
             if isinstance(arg, SubplotSpec):
                 return arg
             else:
+                if not isinstance(arg, Integral):
+                    cbook.warn_deprecated("3.3", message=message)
+                    arg = str(arg)
                 try:
-                    s = str(int(arg))
-                    rows, cols, num = map(int, s)
-                except ValueError as err:
-                    raise ValueError("Single argument to subplot must be a "
-                                     "3-digit integer") from err
+                    rows, cols, num = map(int, str(arg))
+                except ValueError:
+                    raise ValueError(
+                        f"Single argument to subplot must be a three-digit "
+                        f"integer, not {arg}") from None
                 # num - 1 for converting from MATLAB to python indexing
                 return GridSpec(rows, cols, figure=figure)[num - 1]
         elif len(args) == 3:
             rows, cols, num = args
-            rows = int(rows)
-            cols = int(cols)
-            if rows <= 0:
-                raise ValueError(f"Number of rows must be > 0, not {rows}")
-            if cols <= 0:
-                raise ValueError(f"Number of columns must be > 0, not {cols}")
+            if not (isinstance(rows, Integral) and isinstance(cols, Integral)):
+                cbook.warn_deprecated("3.3", message=message)
+                rows, cols = map(int, [rows, cols])
+            gs = GridSpec(rows, cols, figure=figure)
             if isinstance(num, tuple) and len(num) == 2:
-                i, j = map(int, num)
-                return GridSpec(rows, cols, figure=figure)[i-1:j]
+                if not all(isinstance(n, Integral) for n in num):
+                    cbook.warn_deprecated("3.3", message=message)
+                    i, j = map(int, num)
+                else:
+                    i, j = num
+                return gs[i-1:j]
             else:
+                if not isinstance(num, Integral):
+                    cbook.warn_deprecated("3.3", message=message)
+                    num = int(num)
                 if num < 1 or num > rows*cols:
                     raise ValueError(
                         f"num must be 1 <= num <= {rows*cols}, not {num}")
-                # num - 1 for converting from MATLAB to python indexing
-                return GridSpec(rows, cols, figure=figure)[int(num) - 1]
+                return gs[num - 1]   # -1 due to MATLAB indexing.
         else:
             raise TypeError(f"subplot() takes 1 or 3 positional arguments but "
                             f"{len(args)} were given")
diff --git a/lib/matplotlib/tests/test_figure.py b/lib/matplotlib/tests/test_figure.py
@@ -189,9 +189,9 @@ def test_add_subplot_invalid():
     with pytest.raises(ValueError, match='num must be 1 <= num <= 4'):
         fig.add_subplot(2, 2, 5)
 
-    with pytest.raises(ValueError, match='must be a three-digit number'):
+    with pytest.raises(ValueError, match='must be a three-digit integer'):
         fig.add_subplot(42)
-    with pytest.raises(ValueError, match='must be a three-digit number'):
+    with pytest.raises(ValueError, match='must be a three-digit integer'):
         fig.add_subplot(1000)
 
     with pytest.raises(TypeError, match='takes 1 or 3 positional arguments '
@@ -533,3 +533,16 @@ def test_removed_axis():
     fig, axs = plt.subplots(2, sharex=True)
     axs[0].remove()
     fig.canvas.draw()
+
+
+def test_add_subplot_twotuple():
+    fig = plt.figure()
+    ax2 = fig.add_subplot(3, 2, (3, 5))
+    assert ax2.get_subplotspec().rowspan == range(1, 3)
+    assert ax2.get_subplotspec().colspan == range(0, 1)
+    ax3 = fig.add_subplot(3, 2, (4, 6))
+    assert ax3.get_subplotspec().rowspan == range(1, 3)
+    assert ax3.get_subplotspec().colspan == range(1, 2)
+    ax4 = fig.add_subplot(3, 2, (3, 6))
+    assert ax4.get_subplotspec().rowspan == range(1, 3)
+    assert ax4.get_subplotspec().colspan == range(0, 2)
