From 5a5c2f6ade2bb5cf92c9fde50ed07872e6b559f2 Mon Sep 17 00:00:00 2001 From: Eric Firing Date: Sun, 14 Apr 2019 13:30:58 -1000 Subject: [PATCH 1/5] Scatter: make "c" argument handling more consistent. Closes #12735. --- lib/matplotlib/axes/_axes.py | 35 +++++++++++++------------------ lib/matplotlib/tests/test_axes.py | 31 ++++++++++++++++++--------- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py index 60a2cce43d40..e92130b158d8 100644 --- a/lib/matplotlib/axes/_axes.py +++ b/lib/matplotlib/axes/_axes.py @@ -4133,7 +4133,7 @@ def dopatch(xs, ys, **kwargs): medians=medians, fliers=fliers, means=means) @staticmethod - def _parse_scatter_color_args(c, edgecolors, kwargs, xshape, yshape, + def _parse_scatter_color_args(c, edgecolors, kwargs, xsize, get_next_color_func): """ Helper function to process color related arguments of `.Axes.scatter`. @@ -4163,8 +4163,8 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xshape, yshape, Additional kwargs. If these keys exist, we pop and process them: 'facecolors', 'facecolor', 'edgecolor', 'color' Note: The dict is modified by this function. - xshape, yshape : tuple of int - The shape of the x and y arrays passed to `.Axes.scatter`. + xsize : int + The size of the x and y arrays passed to `.Axes.scatter`. get_next_color_func : callable A callable that returns a color. This color is used as facecolor if no other color is provided. @@ -4187,9 +4187,6 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xshape, yshape, The edgecolor specification. """ - xsize = functools.reduce(operator.mul, xshape, 1) - ysize = functools.reduce(operator.mul, yshape, 1) - facecolors = kwargs.pop('facecolors', None) facecolors = kwargs.pop('facecolor', facecolors) edgecolors = kwargs.pop('edgecolor', edgecolors) @@ -4241,9 +4238,9 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xshape, yshape, else: try: # First, does 'c' look suitable for value-mapping? c_array = np.asanyarray(c, dtype=float) - n_elem = c_array.shape[0] - if c_array.shape in [xshape, yshape]: - c = np.ma.ravel(c_array) + n_elem = c_array.size + if n_elem == xsize: + c = c_array.ravel() else: if c_array.shape in ((3,), (4,)): _log.warning( @@ -4263,18 +4260,17 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xshape, yshape, try: # Then is 'c' acceptable as PathCollection facecolors? colors = mcolors.to_rgba_array(c) n_elem = colors.shape[0] - if colors.shape[0] not in (0, 1, xsize, ysize): + if colors.shape[0] not in (0, 1, xsize): # NB: remember that a single color is also acceptable. # Besides *colors* will be an empty array if c == 'none'. valid_shape = False raise ValueError - except ValueError: + except (ValueError, TypeError): if not valid_shape: # but at least one conversion succeeded. raise ValueError( "'c' argument has {nc} elements, which is not " - "acceptable for use with 'x' with size {xs}, " - "'y' with size {ys}." - .format(nc=n_elem, xs=xsize, ys=ysize) + "acceptable for use with 'x' and 'y' with size {xs}." + .format(nc=n_elem, xs=xsize) ) else: # Both the mapping *and* the RGBA conversion failed: pretty @@ -4301,7 +4297,7 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None, Parameters ---------- - x, y : array_like, shape (n, ) + x, y : scalar or array_like, shape (n, ) The data positions. s : scalar or array_like, shape (n, ), optional @@ -4313,8 +4309,8 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None, - A single color format string. - A sequence of color specifications of length n. - - A sequence of n numbers to be mapped to colors using *cmap* and - *norm*. + - A scalar or sequence of n numbers to be mapped to colors using + *cmap* and *norm*. - A 2-D array in which the rows are RGB or RGBA. Note that *c* should not be a single numeric RGB or RGBA sequence @@ -4403,7 +4399,7 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None, plotted. * Fundamentally, scatter works with 1-D arrays; *x*, *y*, *s*, and *c* - may be input as 2-D arrays, but within scatter they will be + may be input as N-D arrays, but within scatter they will be flattened. The exception is *c*, which will be flattened only if its size matches the size of *x* and *y*. @@ -4416,7 +4412,6 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None, # np.ma.ravel yields an ndarray, not a masked array, # unless its argument is a masked array. - xshape, yshape = np.shape(x), np.shape(y) x = np.ma.ravel(x) y = np.ma.ravel(y) if x.size != y.size: @@ -4429,7 +4424,7 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None, c, colors, edgecolors = \ self._parse_scatter_color_args( - c, edgecolors, kwargs, xshape, yshape, + c, edgecolors, kwargs, x.size, get_next_color_func=self._get_patches_for_fill.get_next_color) if plotnonfinite and colors is None: diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py index cff4325f1936..ddaec79c97e1 100644 --- a/lib/matplotlib/tests/test_axes.py +++ b/lib/matplotlib/tests/test_axes.py @@ -1890,6 +1890,21 @@ def test_scatter_no_invalid_color(self, fig_test, fig_ref): ax = fig_ref.subplots() ax.scatter([0, 2], [0, 2], c=[1, 2], s=[1, 3], cmap=cmap) + @check_figures_equal(extensions=["png"]) + def test_scatter_single_point(self, fig_test, fig_ref): + ax = fig_test.subplots() + ax.scatter(1, 1, c=1) + ax = fig_ref.subplots() + ax.scatter([1], [1], c=[1]) + + @check_figures_equal(extensions=["png"]) + def test_scatter_different_shapes(self, fig_test, fig_ref): + x = np.arange(10) + ax = fig_test.subplots() + ax.scatter(x, x.reshape(2, 5), c=x.reshape(5, 2)) + ax = fig_ref.subplots() + ax.scatter(x.reshape(5, 2), x, c=x.reshape(2, 5)) + # Parameters for *test_scatter_c*. NB: assuming that the # scatter plot will have 4 elements. The tuple scheme is: # (*c* parameter case, exception regexp key or None if no exception) @@ -1946,7 +1961,7 @@ def get_next_color(): from matplotlib.axes import Axes - xshape = yshape = (4,) + xsize = 4 # Additional checking of *c* (introduced in #11383). REGEXP = { @@ -1956,21 +1971,18 @@ def get_next_color(): if re_key is None: Axes._parse_scatter_color_args( - c=c_case, edgecolors="black", kwargs={}, - xshape=xshape, yshape=yshape, + c=c_case, edgecolors="black", kwargs={}, xsize=xsize, get_next_color_func=get_next_color) else: with pytest.raises(ValueError, match=REGEXP[re_key]): Axes._parse_scatter_color_args( - c=c_case, edgecolors="black", kwargs={}, - xshape=xshape, yshape=yshape, + c=c_case, edgecolors="black", kwargs={}, xsize=xsize, get_next_color_func=get_next_color) -def _params(c=None, xshape=(2,), yshape=(2,), **kwargs): +def _params(c=None, xsize=2, **kwargs): edgecolors = kwargs.pop('edgecolors', None) - return (c, edgecolors, kwargs if kwargs is not None else {}, - xshape, yshape) + return (c, edgecolors, kwargs if kwargs is not None else {}, xsize) _result = namedtuple('_result', 'c, colors') @@ -2022,8 +2034,7 @@ def get_next_color(): c = kwargs.pop('c', None) edgecolors = kwargs.pop('edgecolors', None) _, _, result_edgecolors = \ - Axes._parse_scatter_color_args(c, edgecolors, kwargs, - xshape=(2,), yshape=(2,), + Axes._parse_scatter_color_args(c, edgecolors, kwargs, xsize=2, get_next_color_func=get_next_color) assert result_edgecolors == expected_edgecolors From 198b3f134ba320064c0e7c57977048ce8579858e Mon Sep 17 00:00:00 2001 From: Eric Firing Date: Sun, 14 Apr 2019 14:18:40 -1000 Subject: [PATCH 2/5] Require 's' arg to be a scalar or match 'x' in size. --- lib/matplotlib/axes/_axes.py | 4 +++- lib/matplotlib/tests/test_axes.py | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py index e92130b158d8..636bca16bc7e 100644 --- a/lib/matplotlib/axes/_axes.py +++ b/lib/matplotlib/axes/_axes.py @@ -4420,7 +4420,9 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None, if s is None: s = (20 if rcParams['_internal.classic_mode'] else rcParams['lines.markersize'] ** 2.0) - s = np.ma.ravel(s) # This doesn't have to match x, y in size. + s = np.ma.ravel(s) + if len(s) not in (1, x.size): + raise ValueError("s must be a scalar, or the same size as x and y") c, colors, edgecolors = \ self._parse_scatter_color_args( diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py index ddaec79c97e1..5fefb3479bb9 100644 --- a/lib/matplotlib/tests/test_axes.py +++ b/lib/matplotlib/tests/test_axes.py @@ -1862,6 +1862,13 @@ def test_scatter_color(self): with pytest.raises(ValueError): plt.scatter([1, 2, 3], [1, 2, 3], color=[1, 2, 3]) + def test_scatter_size_arg_size(self): + x = np.arange(4) + with pytest.raises(ValueError): + plt.scatter(x, x, x[1:]) + with pytest.raises(ValueError): + plt.scatter(x[1:], x[1:], x) + @check_figures_equal(extensions=["png"]) def test_scatter_invalid_color(self, fig_test, fig_ref): ax = fig_test.subplots() From 290c9c05b5d314babc5a9a6890fada95ae46e914 Mon Sep 17 00:00:00 2001 From: Eric Firing Date: Tue, 30 Apr 2019 06:34:15 -1000 Subject: [PATCH 3/5] More consistent naming: n_elem -> csize --- lib/matplotlib/axes/_axes.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py index 636bca16bc7e..7156995a8bcb 100644 --- a/lib/matplotlib/axes/_axes.py +++ b/lib/matplotlib/axes/_axes.py @@ -4226,7 +4226,7 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xsize, # favor of mapping, not rgb or rgba. # Convenience vars to track shape mismatch *and* conversion failures. valid_shape = True # will be put to the test! - n_elem = -1 # used only for (some) exceptions + csize = -1 # Number of colors; used for some exceptions. if (c_was_none or kwcolor is not None or @@ -4238,8 +4238,8 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xsize, else: try: # First, does 'c' look suitable for value-mapping? c_array = np.asanyarray(c, dtype=float) - n_elem = c_array.size - if n_elem == xsize: + csize = c_array.size + if csize == xsize: c = c_array.ravel() else: if c_array.shape in ((3,), (4,)): @@ -4259,8 +4259,8 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xsize, if c_array is None: try: # Then is 'c' acceptable as PathCollection facecolors? colors = mcolors.to_rgba_array(c) - n_elem = colors.shape[0] - if colors.shape[0] not in (0, 1, xsize): + csize = colors.shape[0] + if csize not in (0, 1, xsize): # NB: remember that a single color is also acceptable. # Besides *colors* will be an empty array if c == 'none'. valid_shape = False @@ -4270,7 +4270,7 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xsize, raise ValueError( "'c' argument has {nc} elements, which is not " "acceptable for use with 'x' and 'y' with size {xs}." - .format(nc=n_elem, xs=xsize) + .format(nc=csize, xs=xsize) ) else: # Both the mapping *and* the RGBA conversion failed: pretty From d7db8769164fa7cb67c3eae90ef3c8450ea5b44b Mon Sep 17 00:00:00 2001 From: Eric Firing Date: Wed, 1 May 2019 09:01:29 -1000 Subject: [PATCH 4/5] Revert catching of TypeError along with ValueError --- lib/matplotlib/axes/_axes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py index 7156995a8bcb..492230d69402 100644 --- a/lib/matplotlib/axes/_axes.py +++ b/lib/matplotlib/axes/_axes.py @@ -4265,7 +4265,7 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xsize, # Besides *colors* will be an empty array if c == 'none'. valid_shape = False raise ValueError - except (ValueError, TypeError): + except ValueError: if not valid_shape: # but at least one conversion succeeded. raise ValueError( "'c' argument has {nc} elements, which is not " From c1add442bf1e307674f1c83a307378aca19dc8d4 Mon Sep 17 00:00:00 2001 From: Eric Firing Date: Wed, 1 May 2019 09:35:48 -1000 Subject: [PATCH 5/5] Simplify error messages via f-strings. --- lib/matplotlib/axes/_axes.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py index 492230d69402..c02ae3b9e7c8 100644 --- a/lib/matplotlib/axes/_axes.py +++ b/lib/matplotlib/axes/_axes.py @@ -4268,18 +4268,14 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xsize, except ValueError: if not valid_shape: # but at least one conversion succeeded. raise ValueError( - "'c' argument has {nc} elements, which is not " - "acceptable for use with 'x' and 'y' with size {xs}." - .format(nc=csize, xs=xsize) - ) + f"'c' argument has {csize} elements, which is " + "inconsistent with 'x' and 'y' with size {xsize}.") else: # Both the mapping *and* the RGBA conversion failed: pretty # severe failure => one may appreciate a verbose feedback. raise ValueError( - "'c' argument must be a mpl color, a sequence of mpl " - "colors or a sequence of numbers, not {}." - .format(c) # note: could be long depending on c - ) + f"'c' argument must be a mpl color, a sequence of mpl " + "colors, or a sequence of numbers, not {c}.") else: colors = None # use cmap, norm after collection is created return c, colors, edgecolors