-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Legend edgecolor face #20265
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
Legend edgecolor face #20265
Conversation
It looks like Azure is glitching on two jobs. I presume there must be a way to stop and restart them, but I haven't been able to find it. |
|
||
# orig_handle is a PolyCollection and legend_handle is a Patch. | ||
# Directly set Patch color attributes (must be RGBA tuples). | ||
legend_handle._facecolor = first_color(orig_handle.get_facecolor()) |
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 is more invasive than my change so a couple of questions. 1) why set directly? 2) does this cover more cases, in which case should we add more tests?
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.
- We want all the special-case handling to be done by the PolyCollection, after which we simply hand off the results to the Patch. See https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/patches.py#L249 for another example of this approach.
- I don't know. I think it is more a question of whether enough legend with/without hatching cases are tested than of any expected changes in behavior.
More invasive? You could describe it that way, but I view it as a cleanup and an overall simplification.
…265-on-v3.4.x Backport PR #20265 on branch v3.4.x (Legend edgecolor face)
PR Summary
Simplify the legend handler for PolyCollection.
Alternative to #20260; closes #20258.
PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).