8000 Raise warning if both c and facecolors are used in scatter plot (... and related improvements in the test suite). by NGWi · Pull Request #29130 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Raise warning if both c and facecolors are used in scatter plot (... and related improvements in the test suite). #29130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Jan 3, 2025
Merged

Raise warning if both c and facecolors are used in scatter plot (... and related improvements in the test suite). #29130

Changes from 2 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
5a26bb9
Raise warning if both c and facecolors are used in scatter plot. In t…
NGWi Nov 12, 2024
f1966c2
Corrected flake8 formatting warnings.
NGWi Nov 12, 2024
53a0825
Removed offending whitespace and extra line.
NGWi Nov 13, 2024
a777a15
Merge branch 'issue24404_addwarning' of https://github.com/NGWi/matpl…
NGWi Nov 13, 2024
8031809
Corrected old test that qualified for the new warning.
NGWi Nov 13, 2024
8ae213c
Made message more concise, and straightened old test, to fulfill @tim…
NGWi Nov 18, 2024
49f03f7
Added test.
NGWi Nov 18, 2024
8234066
Corrected flake8 linting objections.
NGWi Nov 18, 2024
38c092a
Added direct test of _parse method to attempt to make Codecov happy.
NGWi Nov 18, 2024
e6e49aa
Added pragma tags for code coverage to ignore unused but untested sim…
NGWi Dec 3, 2024
39e1899
Raise warning if both c and facecolors are used in scatter plot. In t…
NGWi Nov 12, 2024
06c8a76
Corrected flake8 formatting warnings.
NGWi Nov 12, 2024
72138c6
Corrected old test that qualified for the new warning.
NGWi Nov 13, 2024
319b92b
Made message more concise, and straightened old test, to fulfill @tim…
NGWi Nov 18, 2024
e348dfc
Added test.
NGWi Nov 18, 2024
fcf6ce2
Corrected flake8 linting objections.
NG 8000 Wi Nov 18, 2024
9601975
Added direct test of _parse method to attempt to make Codecov happy.
NGWi Nov 18, 2024
a18364c
Added pragma tags for code coverage to ignore unused but untested sim…
NGWi Dec 3, 2024
310a15f
Corrected placement of pragma tags and added 'currently unused' to Je…
NGWi Dec 4, 2024
7c01a7b
Tim's suggestion for naming test's list of colors.
NGWi Dec 4, 2024
fd3d9db
Corrected call to changed variable name.
NGWi Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lib/matplotlib/axes/_axes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4617,6 +4617,15 @@ def _parse_scatter_color_args(c, edgecolors, kwargs, xsize,
if edgecolors is None and not mpl.rcParams['_internal.classic_mode']:
edgecolors = mpl.rcParams['scatter.edgecolors']

# Raise a warning if both `c` and `facecolor` are set (issue #24404).
if c is not None and facecolors is not None:
_api.warn_external(
"You passed both c and facecolor/facecolors for the markers. "
"Matplotlib is ignoring the facecolor "
"in favor of what you specified in c. "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly more concise:

Suggested change
"You passed both c and facecolor/facecolors for the markers. "
"Matplotlib is ignoring the facecolor "
"in favor of what you specified in c. "
"Passing both c and facecolor/facecolors for the markers "
"is ambiguous. facecolor is ignored in in favor of c. "

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for reviewing the comment. Your change is more concise, and I understand the argument that matplotlib developers consider passing both as ambiguous.
However, a user intentionally passing both will likely not understand why it is ambiguous. They probably assumed incorrectly, as the issue's Original Poster did, that c defines both a face color and edge color and that just like adding edge color works, the face color can also be overwritten.

"This behavior may change in the future."
)

c_was_none = c is None
if c is None:
c = (facecolors if facecolors is not None
Expand Down
Loading
0