From d206a0e92a6d0c74f70ac1dd3c0d0bfeeb7465b8 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Mon, 6 Jul 2020 22:57:19 -0400 Subject: [PATCH] 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 #447/ 213459eabb44b4b84bfafc51a9c6964f1e0494cf. closes #17849 Revises #17543 / d86cc2bab8183fd3288ed474e4dfd33e0f018908 Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com> Co-authored-by: Jody Klymak Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com> --- lib/matplotlib/axes/_axes.py | 38 +++++++++++++++++++++++++++++-- lib/matplotlib/tests/test_axes.py | 21 +++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py index a6887fc437a2..c89a5deba851 100644 --- a/lib/matplotlib/axes/_axes.py +++ b/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), diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py index 3c0eb232ebe4..4e2112519af5 100644 --- a/lib/matplotlib/tests/test_axes.py +++ b/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" + )