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.

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

nrnavaneet
Copy link
Contributor

@nrnavaneet nrnavaneet commented Jul 12, 2025

This fix improves how labelcolor='linecolor' works in legends when artists (like step histograms or outline scatter plots) don’t have a visible face colour.

What was wrong:

  • For plots like histtype='step' or scatter with facecolors='none', the code was picking up transparent colours.
  • This caused legend text to show up in white or become invisible.

What I changed:

  • Added checks to skip empty or fully transparent colours before setting the label text colour.
  • Ensured the legend text uses the edge or line colour when face colour is not useful.
  • Updated the logic to avoid index errors and handle all common artist types.
  • A new visual test that covers four different plot types to verify labelcolor='linecolor' behaves correctly across edge-only and filled artists.

Now, legend labels always appear with the correct visible colour — matching the lines or edges in the plots.

Closes [#30298]

@rcomer
Copy link
Member

rcomer commented Jul 12, 2025

Thanks for the PR. I tried your branch with the example code from the issue and got this:

image

We still have no label showing for the top right panel and the one for the bottom right panel is now black. I think they should both be blue.

@nrnavaneet
Copy link
Contributor Author

Thanks for testing this out and for the helpful feedback, @rcomer!

I’ll take a closer look at the color resolution logic, especially around how get_facecolor and get_edgecolor interact in these scenarios. As you noted, switching to or including get_edgecolor might resolve this more cleanly.

I’ll push a follow-up commit shortly to address this, thanks again for the detailed insights!

@nrnavaneet
Copy link
Contributor Author

nrnavaneet commented Jul 12, 2025

Thanks again for the helpful feedback, @rcomer !

I’ve made the changes now. Specifically, I updated the logic to check for transparent or missing facecolors (like in outline scatter plots or step histograms) and fall back to using the edgecolor instead as you suggested. This should fix the issue with missing or incorrect legend label colors in those cases.

You can use the following script to visually confirm that the labels now pick up the expected colors across all four plot types:

Let me know if this works better now!

import matplotlib.pyplot as plt
import numpy as np

fig, axes = plt.subplots(2, 2, figsize=(8, 8))
x = np.random.randn(1000)
y = np.random.randn(1000)

# Top Left: Filled Histogram
axes[0, 0].hist(x, histtype='bar', label="filled hist", color='C0')
axes[0, 0].legend(labelcolor='linecolor')
axes[0, 0].set_title("Filled Histogram")

# Top Right: Step Histogram (edge only)
axes[0, 1].hist(x, histtype='step', label="step hist")
axes[0, 1].legend(labelcolor='linecolor')
axes[0, 1].set_title("Step Histogram")

# Bottom Left: Filled Scatter Plot
axes[1, 0].scatter(x, y, label="filled scatter", color='C2')
axes[1, 0].legend(labelcolor='linecolor')
axes[1, 0].set_title("Filled Scatter Plot")

# Bottom Right: Outline Scatter Plot
axes[1, 1].scatter(x, y, label="outline scatter", facecolors='none', edgecolors='C3')
axes[1, 1].legend(labelcolor='linecolor')
axes[1, 1].set_title("Outline Scatter Plot")

fig.suptitle("Legend Labelcolor='linecolor' – Visual Test")
plt.tight_layout()
plt.show()

@nrnavaneet nrnavaneet changed the title Fix legend labelcolor=‘linecolor’ when facecolor is ‘None’ Fix legend labelcolor=‘linecolor’ to handle all cases, including step plots and transparent markers Jul 12, 2025
Copy link

@lukashergt lukashergt left a comment

Choose a reason for hiding this comment

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

Wow, that was fast. As mentioned in #30298, I'm not sure if generally ignoring alpha=0 is really what we want. See the currently different handling of plot and scatter:

import numpy as np
import matplotlib.pyplot as plt
x = np.linspace(0, 1, 10)
plt.plot(x, 'o', c='None', label="spam")
plt.scatter(x, x, c='None', label="ham")
plt.legend(loc=1, labelcolor='linecolor')
image

Currently in this PR, when c, fc and ec are all 'None', then scatter will default to black text, whereas plot will give transparent text.

I think if all color kwargs are 'None' then we indeed want that transparent text, right? Only when one of fc/ec is 'None', but the other is not, then we want 'linecolor' to pick the correct color.

@lukashergt
Copy link

This might go beyond this PR, but a few more failing examples:

import numpy as np
import matplotlib.pyplot as plt
plt.rcParams['legend.loc'] = 1
plt.rcParams['legend.framealpha'] = 1

x = np.linspace(0, 1, 10)

plt.plot(x, -x-0, 'o', c='None',            label="invisible")
plt.plot(x, -x-1, 'o',   c='b', mfc='None', label="matching handle and label color")
plt.plot(x, -x-2, 'o', mec='r', mfc='None', label="NOT MATCHING HANDLE AND LABEL COLOR!!!")
plt.plot(x, -x-3, 'o',   c='g', mec='None', label="again matching")
plt.plot(x, -x-4, 'o', mfc='m', mec='None', label="AGAIN NOT MATCHING!!!")

plt.scatter(x, -x-5,  c='None',             label="OH NO, i AM VISIBLE!!!")
plt.scatter(x, -x-6,    ec='y',  fc='None', label="yay, I work now")
plt.scatter(x, -x-7,    fc='c',  ec='None', label="yay, I still work")

plt.legend(labelcolor='linecolor')
image

Note the two failing cases 'r' and 'm'. However, note also the perfectly working alternatives 'b' and 'g'.

@nrnavaneet
Copy link
Contributor Author

nrnavaneet commented Jul 13, 2025

Thanks, man, nice examples you’ve got there 😂, they really helped clarify the edge cases! I’m actively working on addressing these inconsistencies (especially the mismatched mfc/mec ones like ‘r’ and ‘m’), and I’ll get back to you once I have a solid fix in place. Appreciate the detailed test cases! 🙌

@nrnavaneet
Copy link
Contributor Author

nrnavaneet commented Jul 13, 2025

Hey! I’ve made the changes as discussed, and all the related tests are now passing

  1. I had to update the colour_getters logic and modify a few tests where the check was comparing the full color array. Since the updated logic uses the first valid color in some cases (like gradients or arrays), I updated those tests accordingly.
  2. I’ve also added the ValueError handling to catch invalid labelcolor inputs like 'not-a-color'.
  3. On top of that, I created several new test functions at the end to cover a wide range of edge cases, including transparent colors, None, mixed facecolors, missing labels, empty color arrays, etc.

I’ve tried to ensure everything is correct, but I might’ve missed something. Feel free to point it out if so!

@nrnavaneet
Copy link
Contributor Author

I accidentally added the test file and removed it. This created the cleanliness error. Im unable to resolve the issue. Any tips or advices would be helpful

Copy link

@lukashergt lukashergt left a comment

Choose a reason for hiding this comment

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

Hey! I’ve made the changes as discussed, and all the related tests are now passing

Thanks for your work! Some comments inline. From a reviewers perspective it would be nice to limit any pure reformatting to a minimum to not distract from the meaningful changes.

I accidentally added the test file and removed it. This created the cleanliness error. Im unable to resolve the issue. Any tips or advices would be helpful

I'm afraid I'm not familiar with this, so can't be of any help there. Maybe @rcomer knows how to handle this?

  1. I had to update the colour_getters logic and modify a few tests where the check was comparing the full color array. Since the updated logic uses the first valid color in some cases (like gradients or arrays), I updated those tests accordingly.

I'm not sure the changing behaviour for multi-colour objects is what we want. I think for those cases they chose the default (black) text colour deliberately. Hence the tests. (see also comments inline)

  1. On top of that, I created several new test functions at the end to cover a wide range of edge cases, including transparent colors, None, mixed facecolors, missing labels, empty color arrays, etc.

Your previous tests (e.g. from 14e0fe0) were much more specific and thus helpful, I think. In these latest tests there is not a single assertion, so these only check whether things pass syntactically, but not whether the legend label colours actually correspond to what we expect them to.

Apart from the above and inline points, I can confirm that my previous examples (#30298, #30299 (comment)) now all work correctly :)

Comment on lines -858 to +860
assert mpl.colors.same_color(text.get_color(), color)
assert mpl.colors.same_color(text.get_color(), 'black')

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

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 :)

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?

@nrnavaneet nrnavaneet force-pushed the fix-legend-labelcolor-bug branch from 8e3a35f to 90d2d45 Compare July 14, 2025 14:24
@nrnavaneet
Copy link
Contributor Author

Hey, ive tried fixing all the issues uve asked, kindly know if im wrong.
Thank you.
@rcomer any help regarding the pr cleanliness would be helpful

@jklymak
Copy link
Member

jklymak commented Jul 14, 2025

@nrnavaneet have a look at a tutorial on squashing your commits. If you've not done it before be sure to make a backup git branch. The stack overflow answers here are useful https://stackoverflow.com/questions/5189560/how-do-i-squash-my-last-n-commits-together

@nrnavaneet
Copy link
Contributor Author

nrnavaneet commented Jul 14, 2025

@jklymak, Sorry new to open source. Still figuring out stuffs. Thanks btw. Appreciate it.

Copy link

@lukashergt lukashergt left a comment

Choose a reason for hiding this comment

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

The tests currently don't properly work (see also inline comments with examples). To properly test this, we need to also compare directly to the artist. I think the attempt to parametrize these tests needlessly complicates our life here. Histograms, scatter plots, and plot plots are not similar enough in their output nor colour handling for that. Instead, I propose the following 3 test functions dedicated to histograms, plot plots, and scatter plots respectively.

tests for histograms

def test_legend_labelcolor_linecolor_histograms():
    x = np.arange(10)
    
    # testing c kwarg for bar, step, and stepfilled histograms
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='bar', color='r', 
                      label=f"red bar hist with a red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_facecolor())
    
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='step', color='g', 
                      label=f"green step hist with a green label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('g')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_edgecolor())
    
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='stepfilled', color='b', 
                      label=f"blue stepfilled hist with a blue label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('b')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_facecolor())
    
    # testing c, fc, and ec combinations for bar histograms
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='bar', color='r', ec='b', 
                      label=f"red bar hist with blue edges and a red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_facecolor())
    
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='bar', fc='r', ec='b', 
                      label=f"red bar hist with blue edges and a red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_facecolor())
    
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='bar', fc='none', ec='b', 
                      label=f"unfilled blue bar hist with a blue label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('b')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_edgecolor())
    
    # testing c, and ec combinations for step histograms
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='step', color='r', ec='b', 
                      label=f"blue step hist with a blue label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('b')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_edgecolor())
    
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='step', ec='b', 
                      label=f"blue step hist with a blue label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('b')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_edgecolor())
    
    # testing c, fc, and ec combinations for stepfilled histograms
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='stepfilled', color='r', ec='b', 
                      label=f"red stepfilled hist, blue edges, red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_facecolor())
    
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='stepfilled', fc='r', ec='b', 
                      label=f"red stepfilled hist, blue edges, red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_facecolor())
    
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='stepfilled', fc='none', ec='b', 
                      label=f"unfilled blue stepfilled hist, blue label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('b')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_edgecolor())
    
    fig, ax = plt.subplots()
    _, _, h = ax.hist(x, histtype='stepfilled', fc='r', ec='none', 
                      label=f"edgeless red stepfilled hist with a red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(h[0].get_facecolor())
    plt.close('all')

test for plot plots

def test_legend_labelcolor_linecolor_plot():
    x = np.arange(5)
    
    # testing line plot
    fig, ax = plt.subplots()
    p = ax.plot(x, c='r', label=f"red line with a red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(p[0].get_color())

    # testing c, fc, and ec combinations for maker plots
    fig, ax = plt.subplots()
    p = ax.plot(x, 'o', c='r', label=f"red circles with a red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(p[0].get_color())
    
    fig, ax = plt.subplots()
    p = ax.plot(x, 'o', c='r', mec='b', label=f"red circles, blue edges, red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(p[0].get_color())
    
    fig, ax = plt.subplots()
    p = ax.plot(x, 'o', mfc='r', mec='b', label=f"red circles, blue edges, red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(p[0].get_markerfacecolor())

    # 'none' cases
    fig, ax = plt.subplots()
    p = ax.plot(x, 'o', mfc='none', mec='b', label=f"blue unfilled circles, blue label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('b')
    assert leg.texts[0].get_color() == mcolors.to_rgba(p[0].get_markeredgecolor())
    
    fig, ax = plt.subplots()
    p = ax.plot(x, 'o', mfc='r', mec='none', label=f"red edgeless circles, red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(p[0].get_markerfacecolor())
    
    fig, ax = plt.subplots()
    p = ax.plot(x, 'o', c='none', label=f"invisible circles with invisible label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == 'none'
    assert leg.texts[0].get_color() == p[0].get_markerfacecolor()
    assert leg.texts[0].get_color() == p[0].get_markeredgecolor()
    assert leg.texts[0].get_color() == p[0].get_color()
    plt.close('all')

test for scatter plots

def test_legend_labelcolor_linecolor_scatter():
    x = np.arange(5)

    # testing c, fc, and ec combinations for scatter plots
    fig, ax = plt.subplots()
    p = ax.scatter(x, x, c='r', label=f"red circles with a red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(p.get_facecolor())

    fig, ax = plt.subplots()
    p = ax.scatter(x, x, c='r', ec='b', label=f"red circles, blue edges, red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(p.get_facecolor())
    
    fig, ax = plt.subplots()
    p = ax.scatter(x, x, fc='r', ec='b', label=f"red circles, blue edges, red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(p.get_facecolor())

    # 'none' cases
    fig, ax = plt.subplots()
    p = ax.scatter(x, x, fc='none', ec='b', label=f"blue unfilled circles, blue label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('b')
    assert leg.texts[0].get_color() == mcolors.to_rgba(p.get_edgecolor())
    
    fig, ax = plt.subplots()
    p = ax.scatter(x, x, fc='r', ec='none', label=f"red edgeless circles, red label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == mcolors.to_rgba('r')
    assert leg.texts[0].get_color() == mcolors.to_rgba(p.get_facecolor())
    
    fig, ax = plt.subplots()
    p = ax.scatter(x, x, c='none', label=f"invisible circles with invisible label")
    leg = ax.legend(loc=1, labelcolor='linecolor')
    assert leg.texts[0].get_color() == 'none'
    plt.close('all')

Comment on lines -858 to +860
assert mpl.colors.same_color(text.get_color(), color)
assert mpl.colors.same_color(text.get_color(), 'black')

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?

Comment on lines -579 to +585
'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'
],

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',
            ],

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

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.

Comment on lines +28 to +29


Choose a reason for hiding this comment

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

Remove unnecessarily added newlines.

Comment on lines +1492 to +1494
"plot", plt.plot,
{'mfc': 'None', 'mec': 'red'},
'red'

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.

Comment on lines +1499 to +1501
"plot", plt.plot,
{'mfc': 'cyan', 'mec': 'None'},
'cyan'

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.

Comment on lines +1520 to +1522
"hist", plt.hist,
{'bins': 10, 'color': 'C1', 'histtype': 'bar'},
'black'

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants