From b1cd0a3d57d849d4c54f85d75f86e13de3efedd9 Mon Sep 17 00:00:00 2001
From: "Adrien F. Vincent" <vincent.adrien@gmail.com>
Date: Tue, 5 Jun 2018 15:33:03 -0700
Subject: [PATCH 1/5] improve c kwarg checking (and error messages) in scatter
---
lib/matplotlib/axes/_axes.py | 47 ++++++++++++++++++++++++++++++------
1 file changed, 39 insertions(+), 8 deletions(-)
diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py
index 9fa6e6650b00..8196257e9de0 100644
--- a/lib/matplotlib/axes/_axes.py
+++ b/lib/matplotlib/axes/_axes.py
@@ -3792,7 +3792,9 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None,
Note that *c* should not be a single numeric RGB or RGBA sequence
because that is indistinguishable from an array of values to be
colormapped. If you want to specify the same RGB or RGBA value for
- all points, use a 2-D array with a single row.
+ all points, use a 2-D array with a single row. Otherwise, value-
+ matching will have precedence in case of a size matching with *x*
+ and *y*.
marker : `~matplotlib.markers.MarkerStyle`, optional, default: 'o'
The marker style. *marker* can be either an instance of the class
@@ -3925,29 +3927,58 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None,
# c is an array for mapping. The potential ambiguity
# with a sequence of 3 or 4 numbers is resolved in
# 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 = 0 # used only for (some) exceptions
+
if c_none or co is not None:
c_array = None
else:
- try:
+ 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 xy_shape:
c = np.ma.ravel(c_array)
else:
+ if c_array.shape in ((3,), (4,)):
+ _log.warning(
+ "'c' kwarg looks like a **single** numeric RGB or "
+ "RGBA sequence, which should be avoided as value-"
+ "mapping will have precedence in case its length "
+ "matches with 'x' & 'y'. Please use a 2-D array "
+ "with a single row if you really want to specify "
+ "the same RGB or RGBA value for all points.")
# Wrong size; it must not be intended for mapping.
+ valid_shape = False
c_array = None
except ValueError:
# Failed to make a floating-point array; c must be color specs.
c_array = None
if c_array is None:
- try:
- # must be acceptable as PathCollection facecolors
+ 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 (1, x.size, y.size):
+ # NB: remember that a single color is also acceptable.
+ valid_shape = False
+ raise ValueError
except ValueError:
- # c not acceptable as PathCollection facecolor
- raise ValueError("c of shape {} not acceptable as a color "
- "sequence for x with size {}, y with size {}"
- .format(c.shape, x.size, y.size))
+ if not valid_shape: # but at least one conversion succeeded.
+ raise ValueError(
+ "'c' kwarg has {nc} elements, which is not acceptable "
+ "for use with 'x' with size {xs}, 'y' with size {ys}."
+ .format(nc=n_elem, xs=x.size, ys=y.size)
+ )
+ # Both the mapping *and* the RGBA conversion failed: pretty
+ # severe failure => one may appreciate a verbose feedback.
+ raise ValueError(
+ "'c' kwarg must either be valid as mpl color(s) or "
+ "as numbers to be mapped to colors. "
+ "Here c = {}." # <- beware, could be long depending on c.
+ .format(c)
+ )
else:
colors = None # use cmap, norm after collection is created
From c77ee585e893e36618606230524cf0c5a2f09f85 Mon Sep 17 00:00:00 2001
From: "Adrien F. Vincent" <vincent.adrien@gmail.com>
Date: Wed, 6 Jun 2018 20:49:59 -0700
Subject: [PATCH 2/5] fix an issue when *c* == 'none'
---
lib/matplotlib/axes/_axes.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py
index 8196257e9de0..dcc6f92dfcb3 100644
--- a/lib/matplotlib/axes/_axes.py
+++ b/lib/matplotlib/axes/_axes.py
@@ -3930,7 +3930,7 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None,
# Convenience vars to track shape mismatch *and* conversion failures.
valid_shape = True # will be put to the test!
- n_elem = 0 # used only for (some) exceptions
+ n_elem = -1 # used only for (some) exceptions
if c_none or co is not None:
c_array = None
@@ -3960,8 +3960,9 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=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 (1, x.size, y.size):
+ if colors.shape[0] not in (0, 1, x.size, y.size):
# 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:
From b68cc513c16189d415e6d50a34d15ed66d253a3a Mon Sep 17 00:00:00 2001
From: "Adrien F. Vincent" <vincent.adrien@gmail.com>
Date: Thu, 7 Jun 2018 21:01:05 -0700
Subject: [PATCH 3/5] factor existing scatter tests into a class
---
lib/matplotlib/tests/test_axes.py | 108 +++++++++++++++---------------
1 file changed, 53 insertions(+), 55 deletions(-)
diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py
index 12197bd8e55f..2af9e5375330 100644
--- a/lib/matplotlib/tests/test_axes.py
+++ b/lib/matplotlib/tests/test_axes.py
@@ -1660,63 +1660,61 @@ def test_hist2d_transpose():
ax.hist2d(x, y, bins=10, rasterized=True)
-@image_comparison(baseline_images=['scatter', 'scatter'])
-def test_scatter_plot():
- fig, ax = plt.subplots()
- data = {"x": [3, 4, 2, 6], "y": [2, 5, 2, 3], "c": ['r', 'y', 'b', 'lime'],
- "s": [24, 15, 19, 29]}
-
- ax.scatter(data["x"], data["y"], c=data["c"], s=data["s"])
-
- # Reuse testcase from above for a labeled data test
- fig, ax = plt.subplots()
- ax.scatter("x", "y", c="c", s="s", data=data)
-
-
-@image_comparison(baseline_images=['scatter_marker'], remove_text=True,
- extensions=['png'])
-def test_scatter_marker():
- fig, (ax0, ax1, ax2) = plt.subplots(ncols=3)
- ax0.scatter([3, 4, 2, 6], [2, 5, 2, 3],
- c=[(1, 0, 0), 'y', 'b', 'lime'],
- s=[60, 50, 40, 30],
- edgecolors=['k', 'r', 'g', 'b'],
- marker='s')
- ax1.scatter([3, 4, 2, 6], [2, 5, 2, 3],
- c=[(1, 0, 0), 'y', 'b', 'lime'],
- s=[60, 50, 40, 30],
- edgecolors=['k', 'r', 'g', 'b'],
- marker=mmarkers.MarkerStyle('o', fillstyle='top'))
- # unit area ellipse
- rx, ry = 3, 1
- area = rx * ry * np.pi
- theta = np.linspace(0, 2 * np.pi, 21)
- verts = np.column_stack([np.cos(theta) * rx / area,
- np.sin(theta) * ry / area])
- ax2.scatter([3, 4, 2, 6], [2, 5, 2, 3],
- c=[(1, 0, 0), 'y', 'b', 'lime'],
- s=[60, 50, 40, 30],
- edgecolors=['k', 'r', 'g', 'b'],
- marker=verts)
-
-
-@image_comparison(baseline_images=['scatter_2D'], remove_text=True,
- extensions=['png'])
-def test_scatter_2D():
- x = np.arange(3)
- y = np.arange(2)
- x, y = np.meshgrid(x, y)
- z = x + y
- fig, ax = plt.subplots()
- ax.scatter(x, y, c=z, s=200, edgecolors='face')
+class TestScatter(object):
+ @image_comparison(baseline_images=['scatter', 'scatter'])
+ def test_scatter_plot(self):
+ fig, ax = plt.subplots()
+ data = {"x": [3, 4, 2, 6], "y": [2, 5, 2, 3],
+ "c": ['r', 'y', 'b', 'lime'], "s": [24, 15, 19, 29]}
+ ax.scatter(data["x"], data["y"], c=data["c"], s=data["s"])
-def test_scatter_color():
- # Try to catch cases where 'c' kwarg should have been used.
- with pytest.raises(ValueError):
- plt.scatter([1, 2], [1, 2], color=[0.1, 0.2])
- with pytest.raises(ValueError):
- plt.scatter([1, 2, 3], [1, 2, 3], color=[1, 2, 3])
+ # Reuse testcase from above for a labeled data test
+ fig, ax = plt.subplots()
+ ax.scatter("x", "y", c="c", s="s", data=data)
+
+ @image_comparison(baseline_images=['scatter_marker'], remove_text=True,
+ extensions=['png'])
+ def test_scatter_marker(self):
+ fig, (ax0, ax1, ax2) = plt.subplots(ncols=3)
+ ax0.scatter([3, 4, 2, 6], [2, 5, 2, 3],
+ c=[(1, 0, 0), 'y', 'b', 'lime'],
+ s=[60, 50, 40, 30],
+ edgecolors=['k', 'r', 'g', 'b'],
+ marker='s')
+ ax1.scatter([3, 4, 2, 6], [2, 5, 2, 3],
+ c=[(1, 0, 0), 'y', 'b', 'lime'],
+ s=[60, 50, 40, 30],
+ edgecolors=['k', 'r', 'g', 'b'],
+ marker=mmarkers.MarkerStyle('o', fillstyle='top'))
+ # unit area ellipse
+ rx, ry = 3, 1
+ area = rx * ry * np.pi
+ theta = np.linspace(0, 2 * np.pi, 21)
+ verts = np.column_stack([np.cos(theta) * rx / area,
+ np.sin(theta) * ry / area])
+ ax2.scatter([3, 4, 2, 6], [2, 5, 2, 3],
+ c=[(1, 0, 0), 'y', 'b', 'lime'],
+ s=[60, 50, 40, 30],
+ edgecolors=['k', 'r', 'g', 'b'],
+ verts=verts)
+
+ @image_comparison(baseline_images=['scatter_2D'], remove_text=True,
+ extensions=['png'])
+ def test_scatter_2D(self):
+ x = np.arange(3)
+ y = np.arange(2)
+ x, y = np.meshgrid(x, y)
+ z = x + y
+ fig, ax = plt.subplots()
+ ax.scatter(x, y, c=z, s=200, edgecolors='face')
+
+ def test_scatter_color(self):
+ # Try to catch cases where 'c' kwarg should have been used.
+ with pytest.raises(ValueError):
+ plt.scatter([1, 2], [1, 2], color=[0.1, 0.2])
+ with pytest.raises(ValueError):
+ plt.scatter([1, 2, 3], [1, 2, 3], color=[1, 2, 3])
def test_as_mpl_axes_api():
From 607b4d6f39587f353795f7805b1db7d7f4f5ded1 Mon Sep 17 00:00:00 2001
From: "Adrien F. Vincent" <vincent.adrien@gmail.com>
Date: Thu, 7 Jun 2018 22:21:56 -0700
Subject: [PATCH 4/5] add an explicit test about the 'c' kwarg for scatter
---
lib/matplotlib/tests/test_axes.py | 58 +++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py
index 2af9e5375330..09e4db65da5d 100644
--- a/lib/matplotlib/tests/test_axes.py
+++ b/lib/matplotlib/tests/test_axes.py
@@ -1716,6 +1716,64 @@ def test_scatter_color(self):
with pytest.raises(ValueError):
plt.scatter([1, 2, 3], [1, 2, 3], color=[1, 2, 3])
+ # Parameters for *test_scatter_c_kwarg*. 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)
+ params_scatter_c_kwarg = [
+ # Single letter-sequences
+ ("rgby", None),
+ ("rgb", "shape"),
+ ("rgbrgb", "shape"),
+ (["rgby"], "conversion"),
+ # Special cases
+ ("red", None),
+ ("none", None),
+ (None, None),
+ (["r", "g", "b", "none"], None),
+ # Non-valid color spec (FWIW, 'jaune' means yellow in French)
+ ("jaune", "conversion"),
+ (["jaune"], "conversion"), # wrong type before wrong size
+ (["jaune"]*4, "conversion"),
+ # Value-mapping like
+ ([0.5]*3, None), # should emit a warning for user's eyes though
+ ([0.5]*4, None), # NB: no warning as matching size allows mapping
+ ([0.5]*5, "shape"),
+ # RGB values
+ ([[1, 0, 0]], None),
+ ([[1, 0, 0]]*3, "shape"),
+ ([[1, 0, 0]]*4, None),
+ ([[1, 0, 0]]*5, "shape"),
+ # RGBA values
+ ([[1, 0, 0, 0.5]], None),
+ ([[1, 0, 0, 0.5]]*3, "shape"),
+ ([[1, 0, 0, 0.5]]*4, None),
+ ([[1, 0, 0, 0.5]]*5, "shape"),
+ # Mix of valid color specs
+ ([[1, 0, 0, 0.5]]*3 + [[1, 0, 0]], None),
+ ([[1, 0, 0, 0.5], "red", "0.0"], "shape"),
+ ([[1, 0, 0, 0.5], "red", "0.0", "C5"], None),
+ ([[1, 0, 0, 0.5], "red", "0.0", "C5", [0, 1, 0]], "shape"),
+ # Mix of valid and non valid color specs
+ ([[1, 0, 0, 0.5], "red", "jaune"], "conversion"),
+ ([[1, 0, 0, 0.5], "red", "0.0", "jaune"], "conversion"),
+ ([[1, 0, 0, 0.5], "red", "0.0", "C5", "jaune"], "conversion"),
+ ]
+
+ @pytest.mark.parametrize('c_case, re_key', params_scatter_c_kwarg)
+ def test_scatter_c_kwarg(self, c_case, re_key):
+ # Additional checking of *c* (introduced in #11383).
+ REGEXP = {"shape": "^'c' kwarg has [0-9]+ elements", # shape mismatch
+ "conversion": "^'c' kwarg must either be valid", # bad vals
+ }
+ x = y = [0, 1, 2, 3]
+ fig, ax = plt.subplots()
+
+ if re_key is None:
+ ax.scatter(x, y, c=c_case, edgecolors="black")
+ else:
+ with pytest.raises(ValueError, match=REGEXP[re_key]):
+ ax.scatter(x, y, c=c_case, edgecolors="black")
+
def test_as_mpl_axes_api():
# tests the _as_mpl_axes api
From e9cdfc0070320843f26ec1b03e9ac494b39f4a46 Mon Sep 17 00:00:00 2001
From: "Adrien F. Vincent" <vincent.adrien@gmail.com>
Date: Fri, 8 Jun 2018 10:09:45 -0700
Subject: [PATCH 5/5] fix naming of 'c' and drop the adjective 'named'
---
lib/matplotlib/axes/_axes.py | 19 ++++++++++---------
lib/matplotlib/tests/test_axes.py | 17 +++++++++--------
2 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py
index dcc6f92dfcb3..69fa011cdb2c 100644
--- a/lib/matplotlib/axes/_axes.py
+++ b/lib/matplotlib/axes/_axes.py
@@ -3878,15 +3878,15 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None,
except ValueError:
raise ValueError("'color' kwarg must be an mpl color"
" spec or sequence of color specs.\n"
- "For a sequence of values to be"
- " color-mapped, use the 'c' kwarg instead.")
+ "For a sequence of values to be color-mapped,"
+ " use the 'c' argument instead.")
if edgecolors is None:
edgecolors = co
if facecolors is None:
facecolors = co
if c is not None:
- raise ValueError("Supply a 'c' kwarg or a 'color' kwarg"
- " but not both; they differ but"
+ raise ValueError("Supply a 'c' argument or a 'color'"
+ " kwarg but not both; they differ but"
" their functionalities overlap.")
if c is None:
if facecolors is not None:
@@ -3943,7 +3943,7 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None,
else:
if c_array.shape in ((3,), (4,)):
_log.warning(
- "'c' kwarg looks like a **single** numeric RGB or "
+ "'c' argument looks like a single numeric RGB or "
"RGBA sequence, which should be avoided as value-"
"mapping will have precedence in case its length "
"matches with 'x' & 'y'. Please use a 2-D array "
@@ -3968,15 +3968,16 @@ def scatter(self, x, y, s=None, c=None, marker=None, cmap=None, norm=None,
except ValueError:
if not valid_shape: # but at least one conversion succeeded.
raise ValueError(
- "'c' kwarg has {nc} elements, which is not acceptable "
- "for use with 'x' with size {xs}, 'y' with size {ys}."
+ "'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=x.size, ys=y.size)
)
# Both the mapping *and* the RGBA conversion failed: pretty
# severe failure => one may appreciate a verbose feedback.
raise ValueError(
- "'c' kwarg must either be valid as mpl color(s) or "
- "as numbers to be mapped to colors. "
+ "'c' argument must either be valid as mpl color(s) "
+ "or as numbers to be mapped to colors. "
"Here c = {}." # <- beware, could be long depending on c.
.format(c)
)
diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py
index 09e4db65da5d..96aa1b09d74a 100644
--- a/lib/matplotlib/tests/test_axes.py
+++ b/lib/matplotlib/tests/test_axes.py
@@ -1716,10 +1716,10 @@ def test_scatter_color(self):
with pytest.raises(ValueError):
plt.scatter([1, 2, 3], [1, 2, 3], color=[1, 2, 3])
- # Parameters for *test_scatter_c_kwarg*. NB: assuming that
- # the scatter plot will have 4 elements. The tuple scheme is:
+ # 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)
- params_scatter_c_kwarg = [
+ params_test_scatter_c = [
# Single letter-sequences
("rgby", None),
("rgb", "shape"),
@@ -1759,12 +1759,13 @@ def test_scatter_color(self):
([[1, 0, 0, 0.5], "red", "0.0", "C5", "jaune"], "conversion"),
]
- @pytest.mark.parametrize('c_case, re_key', params_scatter_c_kwarg)
- def test_scatter_c_kwarg(self, c_case, re_key):
+ @pytest.mark.parametrize('c_case, re_key', params_test_scatter_c)
+ def test_scatter_c(self, c_case, re_key):
# Additional checking of *c* (introduced in #11383).
- REGEXP = {"shape": "^'c' kwarg has [0-9]+ elements", # shape mismatch
- "conversion": "^'c' kwarg must either be valid", # bad vals
- }
+ REGEXP = {
+ "shape": "^'c' argument has [0-9]+ elements", # shape mismatch
+ "conversion": "^'c' argument must either be valid", # bad vals
+ }
x = y = [0, 1, 2, 3]
fig, ax = plt.subplots()