-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Legend shows arrow from annotate #8236 #13034
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
Conversation
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.
👍 Thanks for submitting this PR! Overall it looks good, I have added a couple of comments for code style changes. There are also a couple of test failures that need fixing - let us know if you need any more help with that.
lib/matplotlib/legend_handler.py
Outdated
|
||
HandlerBase.__init__(self, **kwargs) | ||
|
||
def create_artists( |
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.
Could the call aruguments be on multiple lines, but with a maximum line length of 79 characters? (if that makes sense; like
def func(arg1, arg2, arg3, arg4,
arg5)
lib/matplotlib/legend_handler.py
Outdated
handler = HandlerFancyArrowPatch() | ||
handle = orig_handle.arrow_patch | ||
|
||
return handler.create_artists( |
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.
Same as above regarding the function arguments here.
lib/matplotlib/legend.py
Outdated
@@ -32,7 +32,8 @@ | |||
from matplotlib.cbook import silent_list, is_hashable, warn_deprecated | |||
from matplotlib.font_manager import FontProperties | |||
from matplotlib.lines import Line2D | |||
from matplotlib.patches import Patch, Rectangle, Shadow, FancyBboxPatch | |||
from matplotlib.patches import (Patch, Rectangle, Shadow, FancyBboxPatch, | |||
FancyArrowPatch) |
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.
Please align inside of the opening parenthesis.
lib/matplotlib/legend.py
Outdated
# support parasite axes: | ||
if hasattr(ax, 'parasites'): | ||
for axx in ax.parasites: | ||
handles_original += (axx.lines + axx.patches + | ||
axx.collections + axx.containers) | ||
axx.collections + axx.containers+ax.texts) |
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.
shouldn't that be axx.texts
?
@haloxxx this looks quite close, but it'd help if you addressed the comments above. |
Refering to community suggestions
After these few months my request doesn't pass through checks. I've tried to apply all the suggestions, but I'm not sure if I can handle this by myself. |
You have a syntax error. Presumably that is straightforward to fix. Did you try running your example script before pushing the new commits? 😉 |
You may also need to rebase this on current master to pick up the fixes / changes to the doc build. |
I'm going to close this in favour of #23160, which is an updated version of this PR. |
PR Summary
Answer for #8236 issue. Legend automatically handles an arrow from annotate. In fact the code is a limited variant of #10688 . FancyArrowPatch and HandlerAnnotation added. There might be some relicts left from the wider solution (#10688).
Example code:
PR Checklist