10000 Creating_parse_bar_color_args to unify color handling in plt.bar with precedence and sequence support for facecolor and edgecolor by Lucx33 · Pull Request #29133 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Creating_parse_bar_color_args to unify color handling in plt.bar with precedence and sequence support for facecolor and edgecolor #29133

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 14 commits into from
Nov 30, 2024
Merged

Creating_parse_bar_color_args to unify color handling in plt.bar with precedence and sequence support for facecolor and edgecolor #29133

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
cde62aa
Consistent color parameters for bar()
Lucx33 Nov 13, 2024
bca9d43
Fix static issue
Lucx33 Nov 14, 2024
ab2253f
Removed unnecessary to_rgba_array()
Lucx33 Nov 20, 2024
88bc7a1
Fixed flake8
Lucx33 Nov 20, 2024
6193498
Removed edgecolor inheritance
Lucx33 Nov 20, 2024
94db14e
Added test_bar_color_precedence
Lucx33 Nov 24, 2024
f92aa1a
Fix
Lucx33 Nov 24, 2024
4959889
Refactored test_bar_color_precedence
Lucx33 Nov 26, 2024
ef746a3
Update bar
Lucx33 Nov 26, 2024
42f5a71
Update lib/matplotlib/tests/test_axes.py
Lucx33 Nov 26, 2024
314a3dc
Update test_axes.py
Lucx33 Nov 26, 2024
062dda1
Minor change "blue" to "yellow"
Lucx33 Nov 30, 2024
ee98b4f
Fix axes
Lucx33 Nov 30, 2024
08741ef
Minor change "blue" to "yellow"
Lucx33 Nov 30, 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
61 changes: 57 additions & 4 deletions lib/matplotlib/axes/_axes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2320,7 +2320,59 @@
# we do by default and convert dx by itself.
dx = convert(dx)
return dx

Check failure on line 2323 in lib/matplotlib/axes/_axes.py

View workflow job for this annotation

GitHub Actions / flake8 

[flake8] reported by reviewdog 🐶
W293 blank line contains whitespace

Raw Output:
./lib/matplotlib/axes/_axes.py:2323:1: W293 blank line contains whitespace
@staticmethod
def _parse_bar_color_args(kwargs, get_next_color_func):
Copy link
Member
@rcomer rcomer Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

According to #12739 the method for parsing scatter color arguments was made static so that we could have unit tests specifically for that method. Do you plan to write similar tests here? If not, this could be an ordinary method and we would not need to pass the get_next_color_func but just access self._get_patches_for_fill.get_next_color directly.

Edit: looking at the discussion in #12739 I think it is not advisable to write such tests but just test the public bar function.

"""
Helper function to process color-related arguments of `.Axes.bar`.

Argument precedence for facecolors:

- kwargs['facecolor']
- kwargs['color']
- 'b' if in classic mode else the result of ``get_next_color_func()``

Argument precedence for edgecolors:

- kwargs['edgecolor']
- kwargs['color']
- None

Parameters
----------
kwargs : dict
Additional kwargs. If these keys exist, we pop and process them:
'facecolor', 'edgecolor', 'color'
Note: The dict is modified by this function.
get_next_color_func : callable
A callable that returns a color. This color is used as facecolor
if no other color is provided.

Returns
-------
facecolor
The facecolor.
edgecolor
The edgecolor.
"""
color = kwargs.pop('color', None)

facecolor = kwargs.pop('facecolor', color)
edgecolor = kwargs.pop('edgecolor', color)

facecolor = (facecolor if facecolor is not None
else "b" if mpl.rcParams['_internal.classic_mode']

Check failure on line 2364 in lib/matplotlib/axes/_axes.py

View workflow job for this 8000 annotation

GitHub Actions / flake8 

[flake8] reported by reviewdog 🐶
E128 continuation line under-indented for visual indent

Raw Output:
./lib/matplotlib/axes/_axes.py:2364:18: E128 continuation line under-indented for visual indent
Copy link
Member

Choose a reason for hiding this comment

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

I guess you made "b" the default in classic mode for consistency with scatter. However this is not the established behaviour for bar and setting it to blue is likely the cause of at least some of your test failures. For scatter it will only be there for consistency with old Matplotlib versions. I think we should just use the get_next_color_func here.

else get_next_color_func())

Check failure on line 2365 in lib/matplotlib/axes/_axes.py

View workflow job for this annotation

GitHub Actions / flake8 

[flake8] reported by reviewdog 🐶
E128 continuation line under-indented for visual indent

Raw Output:
./lib/matplotlib/axes/_axes.py:2365:18: E128 continuation line under-indented for visual indent

Check failure on line 2366 in lib/matplotlib/axes/_axes.py

View workflow job for this annotation

GitHub Actions / flake8 

[flake8] reported by reviewdog 🐶
W293 blank line contains whitespace

Raw Output:
./lib/matplotlib/axes/_axes.py:2366:1: W293 blank line contains whitespace
try:
facecolor = mcolors.to_rgba_array(facecolor)
except ValueError as err:
raise ValueError(

Check warning on line 2370 in lib/matplotlib/axes/_axes.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/axes/_axes.py#L2369-L2370 

Added lines #L2369 - L2370 were not covered by tests
"'facecolor' or 'color' argument must be a valid color or sequence of colors."

Check failure on line 2371 in lib/matplotlib/axes/_axes.py

View workflow job for this annotation

GitHub Actions / flake8 

[flake8] reported by reviewdog 🐶
E501 line too long (94 > 88 characters)

Raw Output:
./lib/matplotlib/axes/_axes.py:2371:89: E501 line too long (94 > 88 characters)
) from err

return facecolor, edgecolor

Check failure on line 2375 in lib/matplotlib/axes/_axes.py

View workflow job for this annotation

GitHub Actions / flake8 

[flake8] reported by reviewdog 🐶
W293 blank line contains whitespace

Raw Output:
./lib/matplotlib/axes/_axes.py:2375:1: W293 blank line contains whitespace
@_preprocess_data()
@_docstring.interpd
def bar(self, x, height, width=0.8, bottom=None, *, align="center",
Expand Down Expand Up @@ -2376,6 +2428,9 @@
Other Parameters
----------------
color : :mpltype:`color` or list of :mpltype:`color`, optional
The colors of the bar faces and edges.

facecolor : :mpltype:`color` or list of :mpltype:`color`, optional
The colors of the bar faces.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The colors of the bar faces.
The colors of the bar faces.
If both *color* and *facecolor are given, *facecolor* takes precedence.


edgecolor : :mpltype:`color` or list of :mpltype:`color`, optional
Expand Down Expand Up @@ -2441,10 +2496,8 @@
bar. See :doc:`/gallery/lines_bars_and_markers/bar_stacked`.
"""
kwargs = cbook.normalize_kwargs(kwargs, mpatches.Patch)
color = kwargs.pop('color', None)
if color is None:
color = self._get_patches_for_fill.get_next_color()
edgecolor = kwargs.pop('edgecolor', None)
color, edgecolor = self._parse_bar_color_args(kwargs, self._get_patches_for_fill.get_next_color)

Check failure on line 2499 in lib/matplotlib/axes/_axes.py

View workflow job for this annotation

GitHub Actions / flake8 

[flake8] reported by reviewdog 🐶
E501 line too long (104 > 88 characters)

Raw Output:
./lib/matplotlib/axes/_axes.py:2499:89: E501 line too long (104 > 88 characters)

Check failure on line 2500 in lib/matplotlib/axes/_axes.py

View workflow job for this annotation

GitHub Actions / flake8 

[flake8] reported by reviewdog 🐶
W293 blank line contains whitespace

Raw Output:
./lib/matplotlib/axes/_axes.py:2500:1: W293 blank line contains whitespace
linewidth = kwargs.pop('linewidth', None)
hatch = kwargs.pop('hatch', None)

Expand Down
Loading
0