Cleanup logic and documentation of tripcolor

timhoffm · QuLogic · timhoffm · commit ef81739dfbcc · 2022-03-27T09:49:11.000+02:00
This issues more and more precise warnings on usage errors but does not
change behavior.

Co-authored-by: Elliott Sales de Andrade &lt;quantum.analyst@gmail.com&gt;
diff --git a/lib/matplotlib/tests/test_triangulation.py b/lib/matplotlib/tests/test_triangulation.py
@@ -12,50 +12,65 @@
 from matplotlib.testing.decorators import image_comparison, check_figures_equal
 
 
-x = [-1, 0, 1, 0]
-y = [0, -1, 0, 1]
-triangles = [[0, 1, 2], [0, 2, 3]]
-mask = [False, True]
-
-
-@pytest.mark.parametrize('args, kwargs, expected', [
-    ([x, y], {}, [x, y, None, None]),
-    ([x, y, triangles], {}, [x, y, triangles, None]),
-    ([x, y], dict(triangles=triangles), [x, y, triangles, None]),
-    ([x, y], dict(mask=mask), [x, y, None, mask]),
-    ([x, y, triangles], dict(mask=mask), [x, y, triangles, mask]),
-    ([x, y], dict(triangles=triangles, mask=mask), [x, y, triangles, mask]),
-])
-def test_extract_triangulation_params(args, kwargs, expected):
-    other_args = [1, 2]
-    other_kwargs = {'a': 3, 'b': '4'}
-    x_, y_, triangles_, mask_, args_, kwargs_ = \
-        mtri.Triangulation._extract_triangulation_params(
-            args + other_args, {**kwargs, **other_kwargs})
-    x, y, triangles, mask = expected
-    assert x_ is x
-    assert y_ is y
-    assert_array_equal(triangles_, triangles)
-    assert mask_ is mask
-    assert args_ == other_args
-    assert kwargs_ == other_kwargs
+class TestTriangulationParams:
+    x = [-1, 0, 1, 0]
+    y = [0, -1, 0, 1]
+    triangles = [[0, 1, 2], [0, 2, 3]]
+    mask = [False, True]
+
+    @pytest.mark.parametrize('args, kwargs, expected', [
+        ([x, y], {}, [x, y, None, None]),
+        ([x, y, triangles], {}, [x, y, triangles, None]),
+        ([x, y], dict(triangles=triangles), [x, y, triangles, None]),
+        ([x, y], dict(mask=mask), [x, y, None, mask]),
+        ([x, y, triangles], dict(mask=mask), [x, y, triangles, mask]),
+        ([x, y], dict(triangles=triangles, mask=mask), [x, y, triangles, mask]),
+    ])
+    def test_extract_triangulation_params(self, args, kwargs, expected):
+        other_args = [1, 2]
+        other_kwargs = {'a': 3, 'b': '4'}
+        x_, y_, triangles_, mask_, args_, kwargs_ = \
+            mtri.Triangulation._extract_triangulation_params(
+                args + other_args, {**kwargs, **other_kwargs})
+        x, y, triangles, mask = expected
+        assert x_ is x
+        assert y_ is y
+        assert_array_equal(triangles_, triangles)
+        assert mask_ is mask
+        assert args_ == other_args
+        assert kwargs_ == other_kwargs
 
 
 def test_extract_triangulation_positional_mask():
-    global x, y, triangles, mask
     # mask cannot be passed positionally
+    mask = [True]
+    args = [[0, 2, 1], [0, 0, 1], [[0, 1, 2]], mask]
     x_, y_, triangles_, mask_, args_, kwargs_ = \
-        mtri.Triangulation._extract_triangulation_params(x, y, triangles, mask)
+        mtri.Triangulation._extract_triangulation_params(args, {})
     assert mask_ is None
     assert args_ == [mask]
-    # the positional mask has to be catched downstream because this must pass
+    # the positional mask must be caught downstream because this must pass
     # unknown args through
 
 
-del x
-del y
-del triangles
-del mask
+def test_triangulation_init():
+    x = [-1, 0, 1, 0]
+    y = [0, -1, 0, 1]
+    with pytest.raises(ValueError, match="x and y must be equal-length"):
+        mtri.Triangulation(x, [1, 2])
+    with pytest.raises(
+            ValueError,
+            match=r"triangles must be a \(N, 3\) int array, but found shape "
+                  r"\(3,\)"):
+        mtri.Triangulation(x, y, [0, 1, 2])
+    with pytest.raises(
+            ValueError,
+            match=r"triangles must be a \(N, 3\) int array, not 'other'"):
+        mtri.Triangulation(x, y, 'other')
+    with pytest.raises(ValueError, match="found value 99"):
+        mtri.Triangulation(x, y, [[0, 1, 99]])
+    with pytest.raises(ValueError, match="found value -1"):
+        mtri.Triangulation(x, y, [[0, 1, -1]])
 
 
 def test_delaunay():
@@ -223,6 +238,49 @@ def test_tripcolor():
     plt.title('facecolors')
 
 
+def test_tripcolor_color():
+    x = [-1, 0, 1, 0]
+    y = [0, -1, 0, 1]
+    fig, ax = plt.subplots()
+    with pytest.raises(ValueError, match="Missing color parameter"):
+        ax.tripcolor(x, y)
+    with pytest.raises(ValueError, match="The length of C must match either"):
+        ax.tripcolor(x, y, [1, 2, 3])
+    with pytest.raises(ValueError,
+                       match="length of facecolors must match .* triangles"):
+        ax.tripcolor(x, y, facecolors=[1, 2, 3, 4])
+    with pytest.raises(ValueError,
+                       match="'gouraud' .* at the points.* not at the faces"):
+        ax.tripcolor(x, y, facecolors=[1, 2], shading='gouraud')
+    with pytest.raises(ValueError,
+                       match="'gouraud' .* at the points.* not at the faces"):
+        ax.tripcolor(x, y, [1, 2], shading='gouraud')  # faces
+    with pytest.raises(ValueError,
+                       match=r"pass C positionally or facecolors via keyword"):
+        ax.tripcolor(x, y, C=[1, 2, 3, 4])
+
+    # smoke test for valid color specifications (via C or facecolors)
+    ax.tripcolor(x, y, [1, 2, 3, 4])  # edges
+    ax.tripcolor(x, y, [1, 2, 3, 4], shading='gouraud')  # edges
+    ax.tripcolor(x, y, [1, 2])  # faces
+    ax.tripcolor(x, y, facecolors=[1, 2])  # faces
+
+
+def test_tripcolor_warnings():
+    x = [-1, 0, 1, 0]
+    y = [0, -1, 0, 1]
+    C = [0.4, 0.5]
+    fig, ax = plt.subplots()
+    # additional parameters
+    with pytest.warns(UserWarning, match="Additional positional parameters"):
+        ax.tripcolor(x, y, C, 'unused_positional')
+    # facecolors takes precednced over C
+    with pytest.warns(UserWarning, match="Positional parameter C .*no effect"):
+        ax.tripcolor(x, y, C, facecolors=C)
+    with pytest.warns(UserWarning, match="Positional parameter C .*no effect"):
+        ax.tripcolor(x, y, 'interpreted as C', facecolors=C)
+
+
 def test_no_modify():
     # Test that Triangulation does not modify triangles array passed to it.
     triangles = np.array([[3, 2, 0], [3, 1, 0]], dtype=np.int32)
diff --git a/lib/matplotlib/tri/triangulation.py b/lib/matplotlib/tri/triangulation.py
@@ -43,7 +43,9 @@ def __init__(self, x, y, triangles=None, mask=None):
         self.x = np.asarray(x, dtype=np.float64)
         self.y = np.asarray(y, dtype=np.float64)
         if self.x.shape != self.y.shape or self.x.ndim != 1:
-            raise ValueError("x and y must be equal-length 1D arrays")
+            raise ValueError("x and y must be equal-length 1D arrays, but "
+                             f"found shapes {self.x.shape!r} and "
+                             f"{self.y.shape!r}")
 
         self.mask = None
         self._edges = None
@@ -58,13 +60,25 @@ def __init__(self, x, y, triangles=None, mask=None):
         else:
             # Triangulation specified. Copy, since we may correct triangle
             # orientation.
-            self.triangles = np.array(triangles, dtype=np.int32, order='C')
+            try:
+                self.triangles = np.array(triangles, dtype=np.int32, order='C')
+            except ValueError as e:
+                raise ValueError('triangles must be a (N, 3) int array, not '
+                                 f'{triangles!r}') from e
             if self.triangles.ndim != 2 or self.triangles.shape[1] != 3:
-                raise ValueError('triangles must be a (?, 3) array')
+                raise ValueError(
+                    'triangles must be a (N, 3) int array, but found shape '
+                    f'{self.triangles.shape!r}')
             if self.triangles.max() >= len(self.x):
-                raise ValueError('triangles max element is out of bounds')
+                raise ValueError(
+                    'triangles are indices into the points and must be in the '
+                    f'range 0 <= i < {len(self.x)} but found value '
+                    f'{self.triangles.max()}')
             if self.triangles.min() < 0:
-                raise ValueError('triangles min element is out of bounds')
+                raise ValueError(
+                    'triangles are indices into the points and must be in the '
+                    f'range 0 <= i < {len(self.x)} but found value '
+                    f'{self.triangles.min()}')
 
         if mask is not None:
             self.mask = np.asarray(mask, dtype=bool)
diff --git a/lib/matplotlib/tri/tripcolor.py b/lib/matplotlib/tri/tripcolor.py
@@ -21,62 +21,82 @@ def tripcolor(ax, *args, alpha=1.0, norm=None, cmap=None, vmin=None,
     optionally the *triangles* and a *mask*. See `.Triangulation` for an
     explanation of these parameters.
 
+    If neither of *triangulation* or *triangles* are given, the triangulation
+    is calculated on the fly. In this case, it does not make sense to provide
+    colors at the triangle faces via *C* or *facecolors* because there are
+    multiple possible triangulations for a group of points and you don't know
+    which triangles will be constructed.
+
     Parameters
     ----------
     triangulation : `.Triangulation`
         An already created triangular grid.
     x, y, triangles, mask
-        Parameters specifying defining the triangular grid. See
-        `.Triangulation`.
-
-
-    The next argument must be *C*, the array of color values, either
-    one per point in the triangulation if color values are defined at
-    points, or one per triangle in the triangulation if color values
-    are defined at triangles. If there are the same number of points
-    and triangles in the triangulation it is assumed that color
-    values are defined at points; to force the use of color values at
-    triangles use the kwarg ``facecolors=C`` instead of just ``C``.
-
-    *shading* may be 'flat' (the default) or 'gouraud'. If *shading*
-    is 'flat' and C values are defined at points, the color values
-    used for each triangle are from the mean C of the triangle's
-    three points. If *shading* is 'gouraud' then color values must be
-    defined at points.
-
-
-
-    tripcolor(x, y, [triangles], C, [mask=mask], ...)
-
-
-    The remaining kwargs are the same as for `~.Axes.pcolor`.
+        Parameters defining the triangular grid. See `.Triangulation`.
+        This is mutually exclusive with specifying *triangulation*.
+    C : array-like
+        The color values, either for the points or for the triangles. Which one
+        is automatically inferred from the length of *C*, i.e. does it match
+        the number of points or the number of triangles. If there are the same
+        number of points and triangles in the triangulation it is assumed that
+        color values are defined at points; to force the use of color values at
+        triangles use the keyword argument ``facecolors=C`` instead of just
+        ``C``.
+        This parameter is position-only.
+    facecolors : array-like, optional
+        Can be used alternatively to *C* to specify colors at the triangle
+        faces. This parameter takes precedence over *C*.
+    shading : {'flat', 'gouraud'}, default: 'flat'
+        If  'flat' and the color values *C* are defined at points, the color
+        values used for each triangle are from the mean C of the triangle's
+        three points. If *shading* is 'gouraud' then color values must be
+        defined at points.
+    other_parameters
+        All other parameters are the same as for `~.Axes.pcolor`.
+
+    Notes
+    -----
+    It is possible to pass the triangles positionally, i.e.
+    ``tripcolor(x, y, triangles, C, ...)``. However, this is discouraged.
+    For more clarity, pass *triangles* via keyword argument.
     """
     _api.check_in_list(['flat', 'gouraud'], shading=shading)
 
     tri, args, kwargs = Triangulation.get_from_args_and_kwargs(*args, **kwargs)
 
-    # C is the colors array defined at either points or faces (i.e. triangles).
-    # If facecolors is None, C are defined at points.
-    # If facecolors is not None, C are defined at faces.
+    # Parse the color to be in one of (the other variable will be None):
+    # - facecolors: if specified at the triangle faces
+    # - point_colors: if specified at the points
     if facecolors is not None:
-        C = facecolors
+        if args:
+            _api.warn_external(
+                "Positional parameter C has no effect when the keyword "
+                "facecolors is given")
+        point_colors = None
+        if len(facecolors) != len(tri.triangles):
+            raise ValueError("The length of facecolors must match the number "
+                             "of triangles")
     else:
+        # Color from positional parameter C
+        if not args:
+            raise ValueError(
+                "Missing color parameter. Please pass C positionally or "
+                "facecolors via keyword")
+        elif len(args) > 1:
+            _api.warn_external(
+                "Additional positional parameters {args[1:]!r} are ignored")
         C = np.asarray(args[0])
-
-    # If there are a different number of points and triangles in the
-    # triangulation, can omit facecolors kwarg as it is obvious from
-    # length of C whether it refers to points or faces.
-    # Do not do this for gouraud shading.
-    if (facecolors is None and len(C) == len(tri.triangles) and
-            len(C) != len(tri.x) and shading != 'gouraud'):
-        facecolors = C
-
-    # Check length of C is OK.
-    if ((facecolors is None and len(C) != len(tri.x)) or
-            (facecolors is not None and len(C) != len(tri.triangles))):
-        raise ValueError('Length of color values array must be the same '
-                         'as either the number of triangulation points '
-                         'or triangles')
+        if len(C) == len(tri.x):
+            # having this before the len(tri.triangles) comparison gives
+            # precedence to nodes if there are as many nodes as triangles
+            point_colors = C
+            facecolors = None
+        elif len(C) == len(tri.triangles):
+            point_colors = None
+            facecolors = C
+        else:
+            raise ValueError('The length of C must match either the number '
+                             'of points or the number of triangles')
 
     # Handling of linewidths, shading, edgecolors and antialiased as
     # in Axes.pcolor
@@ -97,13 +117,11 @@ def tripcolor(ax, *args, alpha=1.0, norm=None, cmap=None, vmin=None,
 
     if shading == 'gouraud':
         if facecolors is not None:
-            raise ValueError('Gouraud shading does not support the use '
-                             'of facecolors kwarg')
-        if len(C) != len(tri.x):
-            raise ValueError('For gouraud shading, the length of color '
-                             'values array must be the same as the '
-                             'number of triangulation points')
+            raise ValueError(
+                "shading='gouraud' can only be used when the colors "
+                "are specified at the points, not at the faces.")
         collection = TriMesh(tri, **kwargs)
+        colors = point_colors
     else:
         # Vertices of triangles.
         maskedTris = tri.get_masked_triangles()
@@ -112,15 +130,17 @@ def tripcolor(ax, *args, alpha=1.0, norm=None, cmap=None, vmin=None,
         # Color values.
         if facecolors is None:
             # One color per triangle, the mean of the 3 vertex color values.
-            C = C[maskedTris].mean(axis=1)
+            colors = point_colors[maskedTris].mean(axis=1)
         elif tri.mask is not None:
             # Remove color values of masked triangles.
-            C = C[~tri.mask]
+            colors = facecolors[~tri.mask]
+        else:
+            colors = facecolors
 
         collection = PolyCollection(verts, **kwargs)
 
     collection.set_alpha(alpha)
-    collection.set_array(C)
+    collection.set_array(colors)
     _api.check_isinstance((Normalize, None), norm=norm)
     collection.set_cmap(cmap)
     collection.set_norm(norm)
