8000 Backport PR #29130: Raise warning if both c and facecolors are used i… · meeseeksmachine/matplotlib@5357f0d · GitHub
[go: up one dir, main page]
Skip to content

Commit 5357f0d

Browse files
NGWimeeseeksmachine
NGWi
authored and
meeseeksmachine
committed
Backport PR matplotlib#29130: Raise warning if both c and facecolors are used in scatter plot (... and related improvements in the test suite).
1 parent 1a8e664 commit 5357f0d

File tree

3 files changed

+65
-9
lines changed

3 files changed

+65
-9
lines changed

‎lib/matplotlib/axes/_axes.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4675,6 +4675,14 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xsize,
46754675
if edgecolors is None and not mpl.rcParams['_internal.classic_mode']:
46764676
edgecolors = mpl.rcParams['scatter.edgecolors']
46774677

4678+
# Raise a warning if both `c` and `facecolor` are set (issue #24404).
4679+
if c is not None and facecolors is not None:
4680+
_api.warn_external(
4681+
"You passed both c and facecolor/facecolors for the markers. "
4682+
"c has precedence over facecolor/facecolors. "
4683+
"This behavior may change in the future."
4684+
)
4685+
46784686
c_was_none = c is None
46794687
if c is None:
46804688
c = (facecolors if facecolors is not None

‎lib/matplotlib/tests/test_axes.py

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2942,7 +2942,7 @@ def test_scatter_different_shapes(self, fig_test, fig_ref):
29422942

29432943
@pytest.mark.parametrize('c_case, re_key', params_test_scatter_c)
29442944
def test_scatter_c(self, c_case, re_key):
2945-
def get_next_color():
2945+
def get_next_color(): # pragma: no cover
29462946
return 'blue' # currently unused
29472947

29482948
xsize = 4
@@ -3036,7 +3036,7 @@ def _params(c=None, xsize=2, *, edgecolors=None, **kwargs):
30363036
_result(c=['b', 'g'], colors=np.array([[0, 0, 1, 1], [0, .5, 0, 1]]))),
30373037
])
30383038
def test_parse_scatter_color_args(params, expected_result):
3039-
def get_next_color():
3039+
def get_next_color(): # pragma: no cover
30403040
return 'blue' # currently unused
30413041

30423042
c, colors, _edgecolors = mpl.axes.Axes._parse_scatter_color_args(
@@ -3063,7 +3063,7 @@ def get_next_color():
30633063
(dict(color='r', edgecolor='g'), 'g'),
30643064
])
30653065
def test_parse_scatter_color_args_edgecolors(kwargs, expected_edgecolors):
3066-
def get_next_color():
3066+
def get_next_color(): # pragma: no cover
30673067
return 'blue' # currently unused
30683068

30693069
c = kwargs.pop('c', None)
@@ -3075,7 +3075,7 @@ def get_next_color():
30753075

30763076

30773077
def test_parse_scatter_color_args_error():
3078-
def get_next_color():
3078+
def get_next_color(): # pragma: no cover
30793079
return 'blue' # currently unused
30803080

30813081
with pytest.raises(ValueError,
@@ -3085,6 +3085,55 @@ def get_next_color():
30853085
c, None, kwargs={}, xsize=2, get_next_color_func=get_next_color)
30863086

30873087

3088+
# Warning message tested in the next two tests.
3089+
WARN_MSG = (
3090+
"You passed both c and facecolor/facecolors for the markers. "
3091+
"c has precedence over facecolor/facecolors. This behavior may "
3092+
"change in the future."
3093+
)
3094+
# Test cases shared between direct and integration tests
3095+
COLOR_TEST_CASES = [
3096+
('red', 'blue'),
3097+
(['red', 'blue'], ['green', 'yellow']),
3098+
([[1, 0, 0], [0, 1, 0]], [[0, 0, 1], [1, 1, 0]])
3099+
]
3100+
3101+
3102+
@pytest.mark.parametrize('c, facecolor', COLOR_TEST_CASES)
3103+
def test_parse_c_facecolor_warning_direct(c, facecolor):
3104+
"""Test the internal _parse_scatter_color_args method directly."""
3105+
def get_next_color(): # pragma: no cover
3106+
return 'blue' # currently unused
3107+
3108+
# Test with facecolors (plural)
3109+
with pytest.warns(UserWarning, match=WARN_MSG):
3110+
mpl.axes.Axes._parse_scatter_color_args(
3111+
c=c, edgecolors=None, kwargs={'facecolors': facecolor},
3112+
xsize=2, get_next_color_func=get_next_color)
3113+
3114+
# Test with facecolor (singular)
3115+
with pytest.warns(UserWarning, match=WARN_MSG):
3116+
mpl.axes.Axes._parse_scatter_color_args(
3117+
c=c, edgecolors=None, kwargs={'facecolor': facecolor},
3118+
xsize=2, get_next_color_func=get_next_color)
3119+
3120+
3121+
@pytest.mark.parametrize('c, facecolor', COLOR_TEST_CASES)
3122+
def test_scatter_c_facecolor_warning_integration(c, facecolor):
3123+
"""Test the warning through the actual scatter plot creation."""
3124+
fig, ax = plt.subplots()
3125+
x = [0, 1] if isinstance(c, (list, tuple)) else [0]
3126+
y = x
3127+
3128+
# Test with facecolors (plural)
3129+
with pytest.warns(UserWarning, match=WARN_MSG):
3130+
ax.scatter(x, y, c=c, facecolors=facecolor)
3131+
3132+
# Test with facecolor (singular)
3133+
with pytest.warns(UserWarning, match=WARN_MSG):
3134+
ax.scatter(x, y, c=c, facecolor=facecolor)
3135+
3136+
30883137
def test_as_mpl_axes_api():
30893138
# tests the _as_mpl_axes api
30903139
class Polar:
@@ -9064,8 +9113,8 @@ def test_child_axes_removal():
90649113

90659114
def test_scatter_color_repr_error():
90669115

9067-
def get_next_color():
9068-
return 'blue' # pragma: no cover
9116+
def get_next_color(): # pragma: no cover
9117+
return 'blue' # currently unused
90699118
msg = (
90709119
r"'c' argument must be a color, a sequence of colors"
90719120
r", or a sequence of numbers, not 'red\\n'"

‎lib/matplotlib/tests/test_legend.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -970,13 +970,12 @@ def test_legend_pathcollection_labelcolor_markfacecolor_cmap():
970970
# test the labelcolor for labelcolor='markerfacecolor' on PathCollection
971971
# with colormaps
972972
fig, ax = plt.subplots()
973-
facecolors = mpl.cm.viridis(np.random.rand(10))
973+
colors = mpl.cm.viridis(np.random.rand(10))
974974
ax.scatter(
975975
np.arange(10),
976976
np.arange(10),
977977
label='#1',
978-
c=np.arange(10),
979-
facecolor=facecolors
978+
c=colors
980979
)
981980

982981
leg = ax.legend(labelcolor='markerfacecolor')

0 commit comments

Comments
 (0)
0