Skip to content

Fix legend labelcolor=‘linecolor’ to handle various corner cases, e.g. step histograms and transparent markers #30328

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

Merged
merged 12 commits into from
Aug 19, 2025

Conversation

lukashergt
Copy link
Contributor

@lukashergt lukashergt commented Jul 18, 2025

PR summary

This PR addresses issue #30298, which pointed out some corner cases, where labelcolor='linecolor' did not work as expected, e.g.:

  • histograms with histtype='step' or scatter plots with fc='none' had an empty legend label.
  • plots that specified mec and mfc but not c would end up with non-matching handle and label colours.

The failing checks in this test from commit dabaf62 highlight the issue.

Changes:

  • Change 'linecolor' ordering of checks from c, fc to mfc, fc, mec, ec, c.
  • Skip when color is empty, equal to none or with a zero alpha value, e.g. when fc is 'none' continue the loop to check whether there is some color in ec.

Thanks to @nrnavaneet, who worked on this in #30299, and which this PR is based on.

Closes #30298

PR checklist

…o `linecolor`, e.g. ensuring that histograms with `step` histtype do not end up with an empty label
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

…m `c, fc` to `mfc, fc, mec, ec, c`; check if `color` is empty, `none` or transparent and loop on if so
@nrnavaneet
Copy link
Contributor

nrnavaneet commented Jul 18, 2025

Nice work @lukashergt ! The tests are really helpful and good. Glad it was fixed by u :) U deserve the credit.

lukashergt and others added 2 commits July 18, 2025 17:03
Using helper for `'none'` string equality and using the fact that `to_rgba` always returns a 4-element tuple to simplify the code.

Co-authored-by: Tim Hoffmann <[email protected]>
…ble handles; can pass empty string `" "` to achieve empty/invisible label instead; having a black legend label by default allows the creation of neutrally coloured dummy entries
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Minor improvement suggestions.

@lukashergt
Copy link
Contributor Author

I'm pretty sure the current test failure has nothing to do with this PR. Some check timed out. If I could, I would simply try to re-run that test.

Anything else that I should be doing, @timhoffm?

@timhoffm
Copy link
Member

The test failure is unrelated. From my perspective, you don't have to do anything. We just have to wait for a second reviewer approval.

@lukashergt lukashergt requested a review from QuLogic August 18, 2025 15:05
@QuLogic QuLogic merged commit f0fde11 into matplotlib:main Aug 19, 2025
40 checks passed
@QuLogic
Copy link
Member

QuLogic commented Aug 19, 2025

Thanks @lukashergt! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again.

@QuLogic QuLogic modified the milestone: v3.10.6 Aug 19, 2025
@QuLogic QuLogic added this to the v3.11.0 milestone Aug 19, 2025
@QuLogic QuLogic moved this from Waiting for author to Merged in First Time Contributors Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

[Bug]: Legend kwarg labelcolor='linecolor' not working properly when facecolor is 'None'
4 participants