Add textcolor to legend based on labelcolor string #24122
Conversation
As raised by matplotlib#20577, setting `labelcolor` to any of the string options did not work with a scatter plot as it is a PathCollection with possible multiple color values. This commit fixes that. Now, if there is a Colormap or a scatter plot with multiple values, then, the legend text color is not changed, but otherwise, the text color is changed to the color of the scatter markers.
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. 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.
|
@timhoffm as we discussed, I started working on issue #20577. I wanted some suggestions if you could help. Did not want to clutter the issue discussion with code questions so moved to this working PR. My idea is that if we have a fig, ax = plt.subplots()
ax.scatter(np.arange(10), np.arange(10)*1, label='#1', c='r')
ax.scatter(np.arange(10), np.arange(10)*2, label='#2', c='g')
ax.scatter(np.arange(10), np.arange(10)*3, label='#3', c='b')
leg = ax.legend(labelcolor="linecolor")This creates a def test_legend_pathcollection_labelcolor_linecolor():
fig, ax = plt.subplots()
ax.scatter(np.arange(10), np.arange(10)*1, label='#1', c='r')
ax.scatter(np.arange(10), np.arange(10)*2, label='#2', c='g')
ax.scatter(np.arange(10), np.arange(10)*3, label='#3', c='b')
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)the generated cmap comes out to be |
|
I admit this is quite arcane for historic reasons. All collections have a colormap, even if they don't use it. |
Due to inconsistencies in the colormap setup, it is suggested to instead check for `get_array` to get an array of values to be color mapped. This check is more robust irrespective of whatever the type of colormap is set to be.
Tests are added to check various conditions for the text color for a legend set with `linecolor`, `markeredgecolor`, `markerfacecolor` in a scatter plot.
|
@timhoffm Thanks for that! I've updated and added tests to the PR. I was not sure where to document this change though. Let me know what else I need to do? |
|
@timhoffm thanks! I updated the tests. Should be good now :) |
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
|
Right, sorry. Should have spotted that too. Thanks! |
|
@timhoffm changed it to the correct |
|
Thanks @chahak13 and congratulations on your first merged Matplotlib PR 🎉 (I know you have at least one more in flight). I hope we continue to hear from you. Even though this is a bug fix, it is a fair bit of code and I do not feel comfortable backporting this no 3.6.x. |
PR Summary
As raised by #20577, setting
labelcolorto any of the string options did not work with a scatter plot as it is a PathCollection with possible multiple color values. This commit fixes that. Now, if there is a colormap or a scatter plot with multiple values, then, the legend text color is not changed, but otherwise, the text color is changed to the color of the scatter markers.PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).