MNT: use get_fillstyle not is_filled to null facecolor

This is brittle, but matches the behavior in Line2D.

MarkerStyle objects have two coupled, but not fully redundant methods
for determining if the maker is filled: the `is_filled` and
`get_fillstyle` methods.  If `ms.get_fillstyle() == 'none'` then
`ms.is_filled() is False`, however the converse is not True.  In
particular the markers that can not be filled (because the Paths they
are made out of can not be closed) have `ms.get_fillstyle() == 'full'`
and `ms.is_filled() is False`.  In Line2D we filter on the value of
`get_fillstyle` not on `is_filled` so do the same in `Axes.scatter`.

In Line2D we do the validation at draw time (because Line2D holds onto
its MarkerStyle object instead of just extracting the path).

The logic for fillstyle on Markers came in via matplotlib#447/
213459e.

closes matplotlib#17849

Revises matplotlib#17543 / d86cc2b

Co-authored-by: Tim Hoffmann <[email protected]>
Co-authored-by: Jody Klymak <[email protected]>
Co-authored-by: Tim Hoffmann <[email protected]>

Author: 3 people

lib/matplotlib/axes/_axes.py

@@ -4490,6 +4490,10 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None,
                 "s must be a scalar, "
                 "or float array-like with the same size as x and y")
 
+        # get the original edgecolor the user passed before we normalize
+        orig_edgecolor = edgecolors
+        if edgecolors is None:
+            orig_edgecolor = kwargs.get('edgecolor', None)
         c, colors, edgecolors = \
             self._parse_scatter_color_args(
                 c, edgecolors, kwargs, x.size,
@@ -4518,6 +4522,36 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None,
         path = marker_obj.get_path().transformed(
             marker_obj.get_transform())
         if not marker_obj.is_filled():
+            if orig_edgecolor is not None:
+                _api.warn_external(
+                    f"You passed a edgecolor/edgecolors ({orig_edgecolor!r}) "
+                    f"for an unfilled marker ({marker!r}).  Matplotlib is "
+                    "ignoring the edgecolor in favor of the facecolor.  This "
+                    "behavior may change in the future."
+                )
+            # We need to handle markers that can not be filled (like
+            # '+' and 'x') differently than markers that can be
+            # filled, but have their fillstyle set to 'none'.  This is
+            # to get:
+            #
+            #  - respecting the fillestyle if set
+            #  - maintaining back-compatibility for querying the facecolor of
+            #    the un-fillable markers.
+            #
+            # While not an ideal situation, but is better than the
+            # alternatives.
+            if marker_obj.get_fillstyle() == 'none':
+                # promote the facecolor to be the edgecolor
+                edgecolors = colors
+                # set the facecolor to 'none' (at the last chance) because
+                # we can not not fill a path if the facecolor is non-null.
+                # (which is defendable at the renderer level)
+                colors = 'none'
+            else:
+                # if we are not nulling the face color we can do this
+                # simpler
+                edgecolors = 'face'
+
             if linewidths is None:
                 linewidths = rcParams['lines.linewidth']
             elif np.iterable(linewidths):
@@ -4529,8 +4563,8 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None,
 
         collection = mcoll.PathCollection(
                 (path,), scales,
-                facecolors=colors if marker_obj.is_filled() else 'none',
-                edgecolors=edgecolors if marker_obj.is_filled() else colors,
+                facecolors=colors,
+                edgecolors=edgecolors,
                 linewidths=linewidths,
                 offsets=offsets,
                 transOffset=kwargs.pop('transform', self.transData),

lib/matplotlib/tests/test_axes.py

@@ -2145,6 +2145,17 @@ def test_scatter_unfilled(self):
                                                    [0.5, 0.5, 0.5, 1]])
         assert_array_equal(coll.get_linewidths(), [1.1, 1.2, 1.3])
 
+    @pytest.mark.style('default')
+    def test_scatter_unfillable(self):
+        coll = plt.scatter([0, 1, 2], [1, 3, 2], c=['0.1', '0.3', '0.5'],
+                           marker='x',
+                           linewidths=[1.1, 1.2, 1.3])
+        assert_array_equal(coll.get_facecolors(), coll.get_edgecolors())
+        assert_array_equal(coll.get_edgecolors(), [[0.1, 0.1, 0.1, 1],
+                                                   [0.3, 0.3, 0.3, 1],
+                                                   [0.5, 0.5, 0.5, 1]])
+        assert_array_equal(coll.get_linewidths(), [1.1, 1.2, 1.3])
+
     def test_scatter_size_arg_size(self):
         x = np.arange(4)
         with pytest.raises(ValueError, match='same size as x and y'):
@@ -6939,3 +6950,13 @@ def test_patch_bounds():  # PR 19078
     bot = 1.9*np.sin(15*np.pi/180)**2
     np.testing.assert_array_almost_equal_nulp(
         np.array((-0.525, -(bot+0.05), 1.05, bot+0.1)), ax.dataLim.bounds, 16)
+
+
+@pytest.mark.style('default')
+def test_warn_ignored_scatter_kwargs():
+    with pytest.warns(UserWarning,
+                      match=r"You passed a edgecolor/edgecolors"):
+
+        c = plt.scatter(
+            [0], [0], marker="+", s=500, facecolor="r", edgecolor="b"
+        )