Skip to content
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

Fix legend yerr #40777

Merged
merged 8 commits into from
Apr 9, 2021
Merged

Fix legend yerr #40777

merged 8 commits into from
Apr 9, 2021

Conversation

MarcoGorelli
Copy link
Member

@MarcoGorelli MarcoGorelli commented Apr 4, 2021

I spent several hours on this, and I don't think there's a simple solution at the moment (though I'd love to be proved wrong!)

Background:

in Nov 2017, #18222 was opened, in which it was noted that marker info isn't preserved in legends. This is because we were getting legend handles using leg.legendHandles:

handles = leg.legendHandles

Due to an open issue in matplotlib, leg.legendHandles doesn't preserve marker info. So, in #27808, the code was updated to use ax.get_legend_handles_labels:

handle, _ = ax.get_legend_handles_labels()

This preserves marker info but has the problem that it may contain additional objects which aren't visible in the legend. The result of this is the permutations observed in #39522 .

I think that having wrong colours in the legend, as in #39522 and #40044, is far worse than the legend not preserving marker info. Solving everything together doesn't currently seem doable, at least not until matplotlib/matplotlib#11357 is solved in matplotlib.

So my suggestion is to revert some of the changes in #27808, so that at least the colors are right, and when the matplotlib issue is addressed, then the marker info can be preserved.


#39522 on master:

image

on this branch:

image


#40044 on master:

image

on this branch:

image

@MarcoGorelli MarcoGorelli added the Visualization plotting label Apr 4, 2021
@MarcoGorelli MarcoGorelli marked this pull request as draft April 4, 2021 18:54
@MarcoGorelli MarcoGorelli marked this pull request as ready for review April 5, 2021 12:40
@jreback
Copy link
Contributor

jreback commented Apr 5, 2021

is #39522 closed? (or partial)

@jreback jreback added this to the 1.3 milestone Apr 5, 2021
@@ -0,0 +1,45 @@
import pytest

Copy link
Contributor

Choose a reason for hiding this comment

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

can you move other legend tests here? (if we have them)

@MarcoGorelli
Copy link
Member Author

@jreback it's only a partial fix of #39522 , as the marker info isn't preserved

can you move other legend tests here? (if we have them)

Sure, though for ease of review, should I do that in a follow-up PR?

@jreback
Copy link
Contributor

jreback commented Apr 5, 2021

Sure, though for ease of review, should I do that in a follow-up PR?

either way

@MarcoGorelli
Copy link
Member Author

OK, have moved those tests over in this pr @jreback

@jreback jreback merged commit d625880 into pandas-dev:master Apr 9, 2021
@jreback
Copy link
Contributor

jreback commented Apr 9, 2021

thanks @MarcoGorelli very nice!

@MarcoGorelli MarcoGorelli deleted the fix-legend-yerr branch April 9, 2021 07:36
JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: plotting with secondary_y and then plotting another line results in wrong color for (right) line
2 participants