Raise warning if both c and facecolors are used in scatter plot (... and related improvements in the test suite). (#29130)

NGWi · timhoffm · web-flow · commit df49afcdbeac · 2025-01-03T09:34:38.000-05:00
* Raise warning if both c and facecolors are used in scatter plot. In the future we may implement c as edgecolors in that case or cause plot to fail.

* Added direct test of _parse method to attempt to make Codecov happy.

* Added pragma tags for code coverage to ignore unused but untested simplified return.

* Corrected old test that qualified for the new warning.


Co-authored-by: Tim Hoffmann &lt;2836374+timhoffm@users.noreply.github.com&gt;
diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py
@@ -4653,6 +4653,14 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xsize,
         if edgecolors is None and not mpl.rcParams['_internal.classic_mode']:
             edgecolors = mpl.rcParams['scatter.edgecolors']
 
+        # Raise a warning if both `c` and `facecolor` are set (issue #24404).
+        if c is not None and facecolors is not None:
+            _api.warn_external(
+                "You passed both c and facecolor/facecolors for the markers. "
+                "c has precedence over facecolor/facecolors. "
+                "This behavior may change in the future."
+            )
+
         c_was_none = c is None
         if c is None:
             c = (facecolors if facecolors is not None
diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py
@@ -2942,7 +2942,7 @@ def test_scatter_different_shapes(self, fig_test, fig_ref):
 
     @pytest.mark.parametrize('c_case, re_key', params_test_scatter_c)
     def test_scatter_c(self, c_case, re_key):
-        def get_next_color():
+        def get_next_color():   # pragma: no cover
             return 'blue'  # currently unused
 
         xsize = 4
@@ -3036,7 +3036,7 @@ def _params(c=None, xsize=2, *, edgecolors=None, **kwargs):
       _result(c=['b', 'g'], colors=np.array([[0, 0, 1, 1], [0, .5, 0, 1]]))),
      ])
 def test_parse_scatter_color_args(params, expected_result):
-    def get_next_color():
+    def get_next_color():   # pragma: no cover
         return 'blue'  # currently unused
 
     c, colors, _edgecolors = mpl.axes.Axes._parse_scatter_color_args(
@@ -3063,7 +3063,7 @@ def get_next_color():
      (dict(color='r', edgecolor='g'), 'g'),
      ])
 def test_parse_scatter_color_args_edgecolors(kwargs, expected_edgecolors):
-    def get_next_color():
+    def get_next_color():   # pragma: no cover
         return 'blue'  # currently unused
 
     c = kwargs.pop('c', None)
@@ -3075,7 +3075,7 @@ def get_next_color():
 
 
 def test_parse_scatter_color_args_error():
-    def get_next_color():
+    def get_next_color():   # pragma: no cover
         return 'blue'  # currently unused
 
     with pytest.raises(ValueError,
@@ -3085,6 +3085,55 @@ def get_next_color():
             c, None, kwargs={}, xsize=2, get_next_color_func=get_next_color)
 
 
+# Warning message tested in the next two tests.
+WARN_MSG = (
+    "You passed both c and facecolor/facecolors for the markers. "
+    "c has precedence over facecolor/facecolors. This behavior may "
+    "change in the future."
+)
+# Test cases shared between direct and integration tests
+COLOR_TEST_CASES = [
+    ('red', 'blue'),
+    (['red', 'blue'], ['green', 'yellow']),
+    ([[1, 0, 0], [0, 1, 0]], [[0, 0, 1], [1, 1, 0]])
+]
+
+
+@pytest.mark.parametrize('c, facecolor', COLOR_TEST_CASES)
+def test_parse_c_facecolor_warning_direct(c, facecolor):
+    """Test the internal _parse_scatter_color_args method directly."""
+    def get_next_color():   # pragma: no cover
+        return 'blue'  # currently unused
+
+    # Test with facecolors (plural)
+    with pytest.warns(UserWarning, match=WARN_MSG):
+        mpl.axes.Axes._parse_scatter_color_args(
+            c=c, edgecolors=None, kwargs={'facecolors': facecolor},
+            xsize=2, get_next_color_func=get_next_color)
+
+    # Test with facecolor (singular)
+    with pytest.warns(UserWarning, match=WARN_MSG):
+        mpl.axes.Axes._parse_scatter_color_args(
+            c=c, edgecolors=None, kwargs={'facecolor': facecolor},
+            xsize=2, get_next_color_func=get_next_color)
+
+
+@pytest.mark.parametrize('c, facecolor', COLOR_TEST_CASES)
+def test_scatter_c_facecolor_warning_integration(c, facecolor):
+    """Test the warning through the actual scatter plot creation."""
+    fig, ax = plt.subplots()
+    x = [0, 1] if isinstance(c, (list, tuple)) else [0]
+    y = x
+
+    # Test with facecolors (plural)
+    with pytest.warns(UserWarning, match=WARN_MSG):
+        ax.scatter(x, y, c=c, facecolors=facecolor)
+
+    # Test with facecolor (singular)
+    with pytest.warns(UserWarning, match=WARN_MSG):
+        ax.scatter(x, y, c=c, facecolor=facecolor)
+
+
 def test_as_mpl_axes_api():
     # tests the _as_mpl_axes api
     class Polar:
@@ -9046,8 +9095,8 @@ def test_child_axes_removal():
 
 def test_scatter_color_repr_error():
 
-    def get_next_color():
-        return 'blue'  # pragma: no cover
+    def get_next_color():   # pragma: no cover
+        return 'blue'  # currently unused
     msg = (
             r"'c' argument must be a color, a sequence of colors"
             r", or a sequence of numbers, not 'red\\n'"
diff --git a/lib/matplotlib/tests/test_legend.py b/lib/matplotlib/tests/test_legend.py
@@ -970,13 +970,12 @@ def test_legend_pathcollection_labelcolor_markfacecolor_cmap():
     # test the labelcolor for labelcolor='markerfacecolor' on PathCollection
     # with colormaps
     fig, ax = plt.subplots()
-    facecolors = mpl.cm.viridis(np.random.rand(10))
+    colors = mpl.cm.viridis(np.random.rand(10))
     ax.scatter(
         np.arange(10),
         np.arange(10),
         label='#1',
-        c=np.arange(10),
-        facecolor=facecolors
+        c=colors
     )
 
     leg = ax.legend(labelcolor='markerfacecolor')
