Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
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! |
nrnavaneet commentedJul 12, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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! importmatplotlib.pyplotaspltimportnumpyasnpfig,axes=plt.subplots(2,2,figsize=(8,8))x=np.random.randn(1000)y=np.random.randn(1000)# Top Left: Filled Histogramaxes[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 Plotaxes[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 Plotaxes[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() |
lukashergt left a comment
There was a problem hiding this 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 ignoringalpha=0 is really what we want. See the currently different handling ofplot andscatter:
importnumpyasnpimportmatplotlib.pyplotaspltx=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, whenc,fc andec are all'None', thenscatter will default to black text, whereasplot will give transparent text.
I think if all color kwargs are'None' then we indeed want that transparent text, right? Only when one offc/ec is'None', but the other is not, then we want'linecolor' to pick the correct color.
lukashergt commentedJul 13, 2025
This might go beyond this PR, but a few more failing examples: importnumpyasnpimportmatplotlib.pyplotaspltplt.rcParams['legend.loc']=1plt.rcParams['legend.framealpha']=1x=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 |
nrnavaneet commentedJul 13, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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! 🙌 |
Uh oh!
There was an error while loading.Please reload this page.
nrnavaneet commentedJul 13, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 |
lukashergt left a comment
There was a problem hiding this 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?
- 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. from14e0fe0) 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 :)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| 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.
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...
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
nrnavaneetJul 14, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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:
So expecting black as the text colour is clearly wrong. Accordingly this test always failed for me with thecolor to'black' modification. Does this pass for you? What does your plot look like?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 useplt.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 checkingfc beforeec. Thanks@rcomer, that was helpful in understanding test results in#30328.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
8e3a35f to90d2d45CompareHey, ive tried fixing all the issues uve asked, kindly know if im wrong. |
@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 usefulhttps://stackoverflow.com/questions/5189560/how-do-i-squash-my-last-n-commits-together |
nrnavaneet commentedJul 14, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@jklymak, Sorry new to open source. Still figuring out stuffs. Thanks btw. Appreciate it. |
lukashergt left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this 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 and to the legend handle. 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
deftest_legend_labelcolor_linecolor_histograms():x=np.arange(10)# testing c kwarg for bar, step, and stepfilled histogramsfig,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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==leg.get_patches()[0].get_facecolor()assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('g')assertleg.texts[0].get_color()==leg.get_patches()[0].get_edgecolor()assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('b')assertleg.texts[0].get_color()==leg.get_patches()[0].get_facecolor()assertleg.texts[0].get_color()==mcolors.to_rgba(h[0].get_facecolor())# testing c, fc, and ec combinations for bar histogramsfig,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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==leg.get_patches()[0].get_facecolor()assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==leg.get_patches()[0].get_facecolor()assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('b')assertleg.texts[0].get_color()==leg.get_patches()[0].get_edgecolor()assertleg.texts[0].get_color()==mcolors.to_rgba(h[0].get_edgecolor())# testing c, and ec combinations for step histogramsfig,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')assertleg.texts[0].get_color()==mcolors.to_rgba('b')assertleg.texts[0].get_color()==leg.get_patches()[0].get_edgecolor()assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('b')assertleg.texts[0].get_color()==leg.get_patches()[0].get_edgecolor()assertleg.texts[0].get_color()==mcolors.to_rgba(h[0].get_edgecolor())# testing c, fc, and ec combinations for stepfilled histogramsfig,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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==leg.get_patches()[0].get_facecolor()assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==leg.get_patches()[0].get_facecolor()assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('b')assertleg.texts[0].get_color()==leg.get_patches()[0].get_edgecolor()assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==leg.get_patches()[0].get_facecolor()assertleg.texts[0].get_color()==mcolors.to_rgba(h[0].get_facecolor())plt.close('all')
test for plot plots
deftest_legend_labelcolor_linecolor_plot():x=np.arange(5)# testing line plotfig,ax=plt.subplots()p=ax.plot(x,c='r',label=f"red line with a red label")leg=ax.legend(loc=1,labelcolor='linecolor')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==mcolors.to_rgba(leg.get_lines()[0].get_color())assertleg.texts[0].get_color()==mcolors.to_rgba(p[0].get_color())# testing c, fc, and ec combinations for maker plotsfig,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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==mcolors.to_rgba(leg.get_lines()[0].get_color())assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==mcolors.to_rgba(leg.get_lines()[0].get_color())assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==mcolors.to_rgba(leg.get_lines()[0].get_markerfacecolor())assertleg.texts[0].get_color()==mcolors.to_rgba(p[0].get_markerfacecolor())# 'none' casesfig,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')assertleg.texts[0].get_color()==mcolors.to_rgba('b')assertleg.texts[0].get_color()==mcolors.to_rgba(leg.get_lines()[0].get_markeredgecolor())assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==mcolors.to_rgba(leg.get_lines()[0].get_markerfacecolor())assertleg.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')assertleg.texts[0].get_color()=='none'assertleg.texts[0].get_color()==leg.get_lines()[0].get_markerfacecolor()assertleg.texts[0].get_color()==leg.get_lines()[0].get_markeredgecolor()assertleg.texts[0].get_color()==leg.get_lines()[0].get_color()assertleg.texts[0].get_color()==p[0].get_markerfacecolor()assertleg.texts[0].get_color()==p[0].get_markeredgecolor()assertleg.texts[0].get_color()==p[0].get_color()plt.close('all')
test for scatter plots
deftest_legend_labelcolor_linecolor_scatter():x=np.arange(5)# testing c, fc, and ec combinations for scatter plotsfig,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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==mcolors.to_rgba(leg.legend_handles[0].get_facecolor())assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==mcolors.to_rgba(leg.legend_handles[0].get_facecolor())assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==mcolors.to_rgba(leg.legend_handles[0].get_facecolor())assertleg.texts[0].get_color()==mcolors.to_rgba(p.get_facecolor())# 'none' casesfig,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')assertleg.texts[0].get_color()==mcolors.to_rgba('b')assertleg.texts[0].get_color()==mcolors.to_rgba(leg.legend_handles[0].get_edgecolor())assertleg.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')assertleg.texts[0].get_color()==mcolors.to_rgba('r')assertleg.texts[0].get_color()==mcolors.to_rgba(leg.legend_handles[0].get_facecolor())assertleg.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')assertleg.texts[0].get_color()=='none'plt.close('all')
| 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.
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:
So expecting black as the text colour is clearly wrong. Accordingly this test always failed for me with thecolor to'black' modification. Does this pass for you? What does your plot look like?
Uh oh!
There was an error while loading.Please reload this page.
| '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' | ||
| ], |
There was a problem hiding this comment.
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 previousfc thenec order, this should be:
'linecolor': ['get_markerfacecolor','get_facecolor','get_markeredgecolor','get_edgecolor','get_color', ],
Uh oh!
There was an error while loading.Please reload this page.
| labelcolor = mpl._val_or_rc(mpl._val_or_rc(labelcolor, 'legend.labelcolor'), | ||
| 'text.color') | ||
| 'text.color') |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Remove unnecessarily added newlines.
| "plot", plt.plot, | ||
| {'mfc': 'None', 'mec': 'red'}, | ||
| 'red' |
There was a problem hiding this comment.
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:
Handle and label do not match.
| "plot", plt.plot, | ||
| {'mfc': 'cyan', 'mec': 'None'}, | ||
| 'cyan' |
There was a problem hiding this comment.
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:
Handle and label do not match.
| "hist", plt.hist, | ||
| {'bins': 10, 'color': 'C1', 'histtype': 'bar'}, | ||
| 'black' |
There was a problem hiding this comment.
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?
nrnavaneet commentedJul 17, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Hi@lukashergt, That said, the tests and insights you provided won’t go in vain they’ll definitely make it easier for others to debug and solve the issue. I’ll still be following along and learning from the progress. Thanks a lot again for all your guidance and support! |
lukashergt commentedJul 18, 2025
Hi@nrnavaneet, I hope my comments were not discouraging, that was not my intent. This PR did nicely fix the issue, it was just a question of pinning down the tests. I could not re-open this PR, so I created a new one in#30328, which usesalmost the same fix as the one in here. |
Nono, its completely fine. It was actually really helpful with all the tests and questions. Im glad u did, learnt a lot. Im a new contributor u see, still learning. I coudnt find the exact soln so i thought it would be wise to give let other experienced people like u handle it. |
Uh oh!
There was an error while loading.Please reload this page.
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]