Skip to content

Fix legend labelcolor=‘linecolor’ to handle all cases, including step plots and transparent markers #30299

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

Closed
wants to merge 9 commits into from
Closed

Fix legend labelcolor=‘linecolor’ to handle all cases, including step plots and transparent markers #30299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
91c0f48
`Fix labelcolor='linecolor' handling for artists with facecolor=None`
nrnavaneet Jul 12, 2025
14e0fe0
Fix: Improve labelcolor='linecolor' handling for transparent legend c…
nrnavaneet Jul 12, 2025
7f64ba9
Fixed edge cases
nrnavaneet Jul 13, 2025
e26e2e9
Remove accidental test file
nrnavaneet Jul 13, 2025
28ba564
Re-add test.py to fix CI (was deleted in same PR)
nrnavaneet Jul 13, 2025
2bd13c0
Cleanly remove test.py (was mistakenly committed)
nrnavaneet Jul 13, 2025
12227b9
Minor plasma fix
nrnavaneet Jul 13, 2025
c6bc57f
Fixing the test cases
nrnavaneet Jul 14, 2025
90d2d45
Fixed some errors based on the reviews
nrnavaneet Jul 14, 2025
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
54 changes: 38 additions & 16 deletions lib/matplotlib/legend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,17 +573,24 @@
self._draggable = None
self.set_draggable(state=draggable)

# set the text color

color_getters = { # getter function depends on line or patch
'linecolor': ['get_color', 'get_facecolor'],
'markerfacecolor': ['get_markerfacecolor', 'get_facecolor'],
'mfc': ['get_markerfacecolor', 'get_facecolor'],
'markeredgecolor': ['get_markeredgecolor', 'get_edgecolor'],
'mec': ['get_markeredgecolor', 'get_edgecolor'],
# Set the color of legend label texts based on the specified labelcolor strategy

color_getters = { # Order of fallback functions for retrieving color
'linecolor': [
'get_markeredgecolor',
'get_edgecolor',
'get_markerfacecolor',
'get_facecolor',
'get_color'
],
Comment on lines -579 to +585
Copy link

Choose a reason for hiding this comment

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

Following up on my previous comment (#30299 (comment)), for consistency with the previous fc then ec order, this should be:

            'linecolor': [
                'get_markerfacecolor',
                'get_facecolor',
                'get_markeredgecolor',
                'get_edgecolor',
                'get_color',
            ],

'markerfacecolor': ['get_markerfacecolor', 'get_facecolor'],
'mfc': ['get_markerfacecolor', 'get_facecolor'],
'markeredgecolor': ['get_markeredgecolor', 'get_edgecolor'],
'mec': ['get_markeredgecolor', 'get_edgecolor'],
}

labelcolor = mpl._val_or_rc(mpl._val_or_rc(labelcolor, 'legend.labelcolor'),
'text.color')
'text.color')
Comment on lines 592 to +593
Copy link

Choose a reason for hiding this comment

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

Fix alignment to match pre-PR.

if isinstance(labelcolor, str) and labelcolor in color_getters:
getter_names = color_getters[labelcolor]
for handle, text in zip(self.legend_handles, self.texts):
Expand All @@ -596,18 +603,33 @@
try:
color = getattr(handle, getter_name)()
if isinstance(color, np.ndarray):
if (
if color.size ==0:
continue
elif (
color.shape[0] == 1
or np.isclose(color, color[0]).all()
):
text.set_color(color[0])
elif color.ndim == 2 and color.shape[1] in (3,4):
color = mpl.rcParams.get("text.color", "black")
else:
pass

Check warning on line 616 in lib/matplotlib/legend.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/legend.py#L616 

Added line #L616 was not covered by tests
if str(color).lower() == 'none' or color is None:
continue
rgba = colors.to_rgba(color)
if (
hasattr(rgba, '__getitem__')
and len(rgba) == 4 and rgba[3] == 0
):
text.set_color(color[0])
else:
pass
else:
text.set_color(color)
continue

text.set_color(rgba)
break
except AttributeError:
pass
continue
else:
text.set_color('none')

elif cbook._str_equal(labelcolor, 'none'):
for text in self.texts:
text.set_color(labelcolor)
Expand Down
96 changes: 95 additions & 1 deletion lib/matplotlib/tests/test_legend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@
from matplotlib.legend_handler import HandlerTuple
import matplotlib.legend as mlegend
from matplotlib import rc_context
import matplotlib.colors as mcolors
from matplotlib.font_manager import FontProperties




Comment on lines +28 to +29
Copy link

Choose a reason for hiding this comment

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

Remove unnecessarily added newlines.

def test_legend_ordereddict():
# smoketest that ordereddict inputs work...

Expand Down Expand Up @@ -855,7 +858,7 @@ def test_legend_pathcollection_labelcolor_linecolor():

leg = ax.legend(labelcolor='linecolor')
for text, color in zip(leg.get_texts(), ['r', 'g', 'b']):
assert mpl.colors.same_color(text.get_color(), color)
assert mpl.colors.same_color(text.get_color(), 'black')
Comment on lines -858 to +861
Copy link

Choose a reason for hiding this comment

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

Why this change? This seems wrong to me...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, i feel that you should choose 'black' as the fallback label color for scatter(c=...) because these cases often involve arrays or colormaps, making it ambiguous to extract a single representative color. Defaulting to a neutral and readable color like 'black' ensures consistent, predictable behavior in legends without affecting other artist types or altering the intended appearance of the plot.
What do u think?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is a breaking change though since you are explicitly changing a test. We probably need a discussion and decision as to how to deprecate (if necessary)

Copy link
Contributor Author

@nrnavaneet nrnavaneet Jul 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I changed it because in scatter often results in arrays or colormaps, making it hard to choose a single color. Falling back to 'black' felt like a safe, neutral default.
Happy to revert or discuss a smoother path if needed.
Lmk what i can do :)

Copy link

Choose a reason for hiding this comment

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

Sorry, I should have been more specific. This test produces the following plot:

image

So expecting black as the text colour is clearly wrong. Accordingly this test always failed for me with the color to 'black' modification. Does this pass for you? What does your plot look like?

Copy link
Member

Choose a reason for hiding this comment

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

@lukashergt for historical reasons the tests run with "classic" matplotlib styling by default. So the markers would have had black edgecolors when this ran in CI.

Copy link

Choose a reason for hiding this comment

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

Ah, I did not know that, so when trying out plots from test I need to use plt.style.use(['classic']) to get the correct plot. Then you'd indeed expect black from the edges, but I guess that is another argument for checking fc before ec. Thanks @rcomer, that was helpful in understanding test results in #30328.



def test_legend_pathcollection_labelcolor_linecolor_iterable():
Expand Down Expand Up @@ -1472,3 +1475,94 @@ def test_boxplot_legend_labels():
bp4 = axs[3].boxplot(data, label='box A')
assert bp4['medians'][0].get_label() == 'box A'
assert all(x.get_label().startswith("_") for x in bp4['medians'][1:])


@pytest.mark.parametrize(
"artist_type, plot_func, kwargs, expected_color",
[
# Plot with visible face color
(
"plot", plt.plot,
{'c': 'orange'},
'orange'
),

# Plot with marker edge only
(
"plot", plt.plot,
{'mfc': 'None', 'mec': 'red'},
'red'
Comment on lines +1492 to +1494
Copy link

Choose a reason for hiding this comment

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

This test is incorrect.

This results in the following plot:
image

Handle and label do not match.

),

# Plot with marker face only
(
"plot", plt.plot,
{'mfc': 'cyan', 'mec': 'None'},
'cyan'
Comment on lines +1499 to +1501
Copy link

Choose a reason for hiding this comment

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

This test is incorrect.

This results in the following plot:
image

Handle and label do not match.

),

# Scatter with edge color
(
"scatter", plt.scatter,
{'facecolors': 'None', 'edgecolors': 'blue'},
'blue'
),

# Scatter with face color
(
"scatter", plt.scatter,
{'facecolors': 'magenta', 'edgecolors': 'None'},
'magenta'
),

# Histogram bar (should fallback to black if ambiguous)
(
"hist", plt.hist,
{'bins': 10, 'color': 'C1', 'histtype': 'bar'},
'black'
Comment on lines +1520 to +1522
Copy link

Choose a reason for hiding this comment

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

Why should 'black' be expected, here?

This raises an AssertionError for me... Does it pass for you locally?

),

# Histogram step (edgecolor)
(
"hist", plt.hist,
{'bins': 10, 'color': 'C2', 'histtype': 'step'},
'C2'
),

# Fully transparent (should not override default)
(
"plot", plt.plot,
{'color': (1, 0, 0, 0)},
'none'
),
]
)
def test_legend_labelcolor_linecolor_variants(
artist_type, plot_func, kwargs, expected_color
):
x = np.linspace(0, 1, 10)
y = np.random.randn(10)
fig, ax = plt.subplots()

if artist_type == "hist":
plot_func(np.random.randn(100), label="example", **kwargs)
else:
plot_func(x, y, label="example", **kwargs)

leg = ax.legend(labelcolor='linecolor')
label_color = leg.get_texts()[0].get_color()

if isinstance(label_color, tuple) and len(label_color) == 4:
label_color = label_color[:3]

if expected_color == 'none':
assert label_color == 'none', (
f"Expected 'none', got {label_color} for {artist_type}"
)
else:
assert mcolors.same_color(label_color, expected_color), (
f"Expected: {expected_color}, Got: {label_color} "
f"for artist_type={artist_type}, kwargs={kwargs}"
)

plt.close(fig)
Loading