Fix legend labelcolor=‘linecolor’ to handle all cases, including step plots and transparent markers #30299
Conversation
|
Thanks for the PR. I tried your branch with the example code from the issue and got this: 
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. |
|
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! |
|
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() |
There was a problem hiding this comment.
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')
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.
|
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')
Note the two failing cases |
|
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! 🙌 |
|
Hey! I’ve made the changes as discussed, and all the related tests are now passing
I’ve tried to ensure everything is correct, but I might’ve missed something. Feel free to point it out if so! |
|
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 |
There was a problem hiding this comment.
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?
- 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)
- 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 :)
| assert mpl.colors.same_color(text.get_color(), color) | ||
| assert mpl.colors.same_color(text.get_color(), 'black') |
There was a problem hiding this comment.
Why this change? This seems wrong to me...
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:
What I changed:
Now, legend labels always appear with the correct visible colour — matching the lines or edges in the plots.
Closes [#30298]