-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Support placing legend symbols next to axis labels #16005
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
base: main
Are you sure you want to change the base?
Conversation
This seems like an interesting addition, but I'm pretty concerned with bringing It would be good to know how this interacts with We already have the idea of legends being outside the axes bounds, so it seems this could just be a modification of those legends. Can you just rotate normal legends 90 degrees? Thats the missing piece here. |
It seems like a lot of work to adapt an ordinary legend to replace an axis label. Also, no, at the moment there are problems with rotating those. Actually, on SO the preferred method to achieve this seems to be to emulate the legends symbols with LaTex. Could you elaborate further which problems you see? The logic could alternatively be placed in I tried with |
This PR basically says that every It seems the less intrusive change is to change the |
Did not look into the details how Anyway, this is a significant API change and needs careful design. |
It would be possible to implement it in What could be changed easily is not attaching a legend handle to a |
I think it is a good idea to move the functionality to a subclass of
So to me forcing legend entries and axis legends into the same object seems to be a lot of additional effort and might even introduce backward compatibility issues, while the joint functionality is quite limited. |
I've moved the logic to a subclass of Text (as far as possible without affecting functionality). Let me know what you think. I'll fix conflicts when I get your ok. |
Rather than moving the functionality to a subclass of Text (given that we just deprecated and removed the somewhat similar TextWithDash #13548 #15950), I would hope that it would be possible to adjust the xylabel machinery to use any (most) Artist instead of just a Text object -- specifically, one could then pass an HPacker (similarly to how legends are built). IOW, use composition (an HPacker which holds the subartists we want, and should be quite more adaptable) rather than inheritance. |
The problem with this approach occurs when allowing multiple lines. Let's consider an example: a) is my current implementation, b) is what you might get when using a Which option do you prefer? To me, b) looks misaligned. You cannot achieve a) with a I'm not opposed to my |
You raise a good point which I didn't think about, but actually now that you show both of them, (b) actually looks correct to me whereas (a) looks misaligned :) Dunno what others think... |
This came up on the dev call this week: https://hackmd.io/4zWKhuLXQ16Y_oUfi9hnKA#16005-legend-on-xlabelylabel . If that time is plausible for you @alexharti it may be productive for you to join I have not had time to carefully review the code so I am responding to the discussion here and a skimming of the code. I think that this is an interesting and useful feature, however I am not convinced that it is something we want to put into the I think the best path forward here is:
The end result would be usage that looks something like (this is a re-phrasing / combining of ideas that have already come up in this thread): import matplotlib.pyplot as plt
from matplotlib.legend import legend_label # TODO better name?
fig, ax = plt.subplots()
ln, = ax.plot(...)
ax.set_ylabel(legend_label(ln, 'my text')) which is only slightly more verbose than current proposal from the end-user point of view. This would also allow there to be out-of-core exploration of While I understand our conservativeness may be frustrating from a contributor stand point, hopefully as a user you have some sympathy for it. Once we put something out, it is very hard to take back or change. |
This certainly is an interesting interface. However, I think there are issues:
As counter proposal let me pick up the import matplotlib.pyplot as plt
from matplotlib.legend import label_legend
fig, ax = plt.subplots()
ln, = ax.plot(...)
ax.set_ylabel('my text', symbol=label_legend(ln)) You could manually create a Independent of the interface I would be interested in a few opinions if in the above comparison #16005 (comment) option a) or b) is preferred (or c) make it configurable). |
For the first point, the For the second point, that is more of an oversight in the API that you can not control the text properties other than grabbing the label object and changing it. Having neither set be kwargs, but have a If you make the symbol "special" now you have to make a whole bunch of extra decisions, in the core API, that it is not clear about the layout and how to control it (what is the aspect ratio of the symbol? what is the padding between the text and the symbol? do you rotate opposite directions on the left and right spines? in the label on to the left, the right, above, or below the text?) What does If we do go with the idea of extending def legend_label(art, label, txt_kwargs, legend_kwargs, layout_kwargs):
handle = extract_legend(art, **legend_kwargs)
return SymbolAwareText(label, handle, **txt_kwargs, **layout_kwargs) If there is no collisions between them, you could also imaging having some sorting logic inside so that the outer API stays flat. |
This seems stalled out. @alexhartl we very much appreciate the contribution, and there seemed substantial support for this, but there would need to be a fair bit of discussion about architecting. You can probably do that here, but I've moved this to "Draft" and marked stale. If there is no further action in a month or so, we may just close this for now. Don't be shy about pinging us. Old PRs can get buried. |
Sorry, at some point I got too busy and lost sight of this. I would very much like to see this functionality in matplotlib and I can do the coding if you want. But I think it's better if you guys first set out a plan of how we proceed here. Besides the interface (which after all isn't too important to me), I see at least the following points:
|
Hi again, I'm currently working on a paper where I would again need this functionality. As I said, I don't care too much about the architectural details, so if we could agree on some interface, so that I can develop a PR that would find its way into matplotlib, that would be great. |
@alexhartl Thanks for your patience with this. We discussed extensively on the weekly developer call. Without getting into exactly what you have already done, we think the architecture should be something like:
I think this is relatively close to your current design, but doesn't make all labels have the extra marker by default, and allows future flexibility for other |
Sounds good. In addition to the actual interface, I introduced a superclass of |
can you explain what that superclass does? Does your new LegendText need to inherit from it? Or can those parameter just be sent directly to LegendText, and then Legend just use LegendText instances? |
It holds several parameters ( Additionally, when placing multiple As alternative to a superclass, how about allowing/requiring to instantiate a dummy
This would be less intrusive and constitute a comparable pattern to how you have to instantiate a dummy |
Without checking the details, I thought it would make more sense the other way: Legend.init takes those parameters, and passes them to |
Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it. |
Hi @alexhartl - would you like help to pick this back up? Cheers! |
Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it. |
PR Summary
For
twinx()
plots it is more space-efficient and intuitive to have legend symbols next to the axis labels instead of in a separate box. This PR adds this functionality, providingplt.xlabel_legend()
andplt.ylabel_legend()
functions.Example from docstring:

PR Checklist