-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Improve(?) implementation of secondary_axis. #14463
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
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.
Intriguing idea. Seems problematic to allow plotting on the secondary axes as we will certainly have folks confused about zorder.
Did you check that the new axes locator behaves under resizing the figure?
Resizing works as far as I can tell. Allowing drawing on the secondary axes is mostly just a nice side effect; I'm not suggesting to add a public API there (it would need its own discussion). On the other hand I don't think zorder is that complicated (basically, the children of the secondary axes all get inserted into the main list at the position specified by the secondary axes zorder) -- but again we can discuss this separately. |
ax2.set_ylabel("Label Y2") | ||
|
||
ax.plot([-2, 3, 2]) | ||
|
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.
OK, this is nice. Can this be added as an example in the main spines section as well? Or moved there, with a link to it here? Putting this second in this section makes it look like this is a deprecated way to do it, whereas I think the goal is for it to be the canonical way.
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.
I removed the additions to simple_axisline2 and simple_axisartist1 and just linked them https://matplotlib.org/devdocs/gallery/ticks_and_spines/spine_placement_demo.html#sphx-glr-gallery-ticks-and-spines-spine-placement-demo-py instead.
I think I'll leave the multi-axes example as it is right now because there's no completely equivalent example elsewhere and I don't want to bother about proper explanations for now, but we can always move it there later (e.g. when we kill SubplotZero...).
I would argue that those arrows on top of the spines are closely linked to having the axes through (0,0). Essentially my impression is that there are two kind of graphs around: "Math graphs", that go through zero and have directional symbols on them, and "Engeneers graphs" that look like the default graphs in matplotlib (possibly with or without top and right border). My vote would hence be to keep showcasing zero passing axes with arrows. |
It's not really hard to add arrows to "standard" (non-mpltoolkits) matplotlib graphs (it's just a matter to add an annotate() (or arrow()) call in axes coordinates; https://gist.github.com/joferkington/3845684 / https://3diagramsperpage.wordpress.com/2014/05/25/arrowheads-for-axis-in-matplotlib/). |
The canonical order in such cases is: Provide an alternative, then remove the original, not vice versa. I think the changes to |
I'm not removing the old (and currently only) way to do that (I'm no even removing the examples for them!); I'm just discussing that the changes here will later enable further changes. |
On the call today we (@anntzer and I) discussed this. One idea is to more thoroughly unify twinned axes with secondary axes so that the user would need to remember less about their (reduced) differences. For example, if secondary axes were not children then the handling of zorder would be identical between twinned axes and secondary axes. |
I think you are just suggesting that a secondary axes is just a twinned axes with a special relationship between the parent and the twin. I guess that’s ok but I don’t like the ambiguity. We have lots of ambiguity with twinned axes, shared axes, etc and it all becomes a bit confusing. Currently a secondary axes is just an annotation on the parent axes. You can’t plot on it. That makes it a lot simpler. I’m not sure why we want to move to something more ambiguous just to give the trivial ability to plot in different units. The user already has the forward and inverse functions so it’s not like it’s hard to plot on the parent axes. |
@ImportanceOfBeingErnest I guess you handled your own concern re "math-style axes" via the solution at #17170? :-) |
I guess my concern is superseeded by my comment in #17170, that is, keep showcasing axisartist functionality, but in the context of that module. |
Currently, secondary_xaxis is implemented by adding a child axes with a physical height set to zero and y position set accordingly relative to its parent axes (using the axes_locator mechanism). This patch changes it so that the child axes' extents actually *matches* the parent axes, and instead positioning the spines using Spine.set_position. It also makes sure that the secondary axes patch is invisible and that the bounds of the "orthogonal" axis (the one that is not shown) matches the parent bounds. By doing so, it becomes possible to plot data directly on the secondary axis as well; e.g. the following now works: ``` from matplotlib import pyplot as plt from matplotlib.axes import Axes fig, ax0 = plt.subplots() ax1 = ax0.secondary_xaxis( "top", functions=(lambda x: x**3, lambda x: x**(1/3))) Axes.plot(ax1, [.25, .5, .75], [.25, .5, .75], ".-") plt.show() ``` (We have to use Axes.plot here instead of ax1.plot just because SecondaryAxes inherits from _AxesBase, not from Axes, but that doesn't really matter.) Another advantage is that one can now use secondary_axis as a replacement for SubplotZero, a relatively obscure feature of mpl_toolkits that is only showcased in 3 examples: https://matplotlib.org/gallery/axisartist/simple_axisline.html https://matplotlib.org/gallery/axisartist/simple_axisline2.html https://matplotlib.org/gallery/axisartist/demo_axisline_style.html whose goal is just to draw a spine at x=0 or y=0. simple_axisline2 just moves its main spine to y=0, so we can implement that directly with Spine.set_position (see also simple_axisartist1). simple_axisline adds additional spines, and I added an equivalent implementation using secondary_axis, which is fairly transparent. (This example, as well as test_secondary_xy, show why the axes patch must be made invisible: otherwise, the patch of later secondary axes would be drawn over other secondary axes.) demo_axisline_style doesn't showcase anything that's specifically related to SubplotZero so I just rewrote it without SubplotZero. If we agree that secondary_axis is a suitable replacement, SubplotZero could ultimately be deprecated (in a later PR). Minor points: Also delete `SecondaryAxis._loc`, which is set in a single place and never used.
I'm still mildly against this. I think secondary_axis is just an axis, not a full fledged axes you can plot on. Folks should use twin for an axes. |
Added this to the weekly call agenda, should hopefully be relatively quick to discuss and reach a decision. |
Sure, but can you re-iterate why it should be an axes versus just using |
Because this way you can write something like fig, ax_celsius = plt.subplots()
ax_farenheit = ax0.secondary_yaxis(
"top", functions=(celsius_to_farenheit, farenheit_to_celsius))
ax_celsius.plot(celsius_data)
Axes.plot(ax_farenheit, farenheit_data)
plt.show() and have everything just work? (Note that I would not argue to add that feature if it had to be implemented from scratch; the point is that all the required machinery basically already there so it seems a pity not to make it available.) |
Sure, but isn't that what |
twinx() doesn't even ensure that the celsius data will be correctly readable on the farenheit axis? |
OK, but then should this replace twinx? Should twinx gain this functionality? My original idea with secondary_axis was for a lightweight annotation artist that didn't confuse people like the existing twin method did; if I could have not had an axes created at all, I'd have done so, but unfortunately an Axis doesn't contain all the artists for some reason. But if we can make this clean I'm not opposed. We could also come up with another API for the two things. |
For me, twinx means you're plotting two unrelated datasets, whose only common point is that they share the independent variable (e.g. temperature vs time and pressure vs time). secondary_axis means that you're plotting datasets that are directly comparable, except that they may be expressed in various units (e.g. celsius vs. farenheit).
Agreed I'm not particularly wedded to the API, again my main point was that this is basically already available as is. |
OK, I think that makes sense. Does the drawing happen on the parent or the child? If we passed add_artist to self.parent.add_artist, that would maybe take care of a lot of the confusion of having two axes. OTOH it wouldn't allow two sets of grids, which folks have asked for in the past. |
Right now it happens on the child, but perhaps forwarding add_artist to the parent may indeed be better. |
I think this brings up issues of what the cursor should show, etc, but I think by stating one is the parent, and one is the child, we can just say the cursor follows the parent regardless of which method the artist was added with. I guess I think this should be a different method. We could even consider soft-deprecating secondary_axis in favour of |
Discussed on call. Agreement was that this would be nice to have, but we need to make sure everything is well lined up. Questions are listed in the summary of the discussion, but among the points I need to look at: 1) does the orthogonal direction get correctly synced? 2) how much implementation can be shared between this and twinx/y? 3) can we pull in parasite_axes functionality, i.e. both axes' artists get z-ordered together, they share the same color cycle? (also legend() works, etc.) |
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. |
Looks like the agreement from the call 2y ago was to try moving this forward. |
Currently, secondary_xaxis is implemented by adding a child axes with a
physical height set to zero and y position set accordingly relative to
its parent axes (using the axes_locator mechanism).
This patch changes it so that the child axes' extents actually
matches the parent axes, and instead positioning the spines using
Spine.set_position. It also makes sure that the secondary axes patch is
invisible and that the bounds of the "orthogonal" axis (the one that is
not shown) matches the parent bounds.
By doing so, it becomes possible to plot data directly on the secondary
axis as well; e.g. the following now works:
(We have to use Axes.plot here instead of ax1.plot just because
SecondaryAxes inherits from _AxesBase, not from Axes, but that doesn't
really matter.)
Another advantage is that one can now use secondary_axis as a
replacement for SubplotZero, a relatively obscure feature of
mpl_toolkits that is only showcased in 3 examples:
https://matplotlib.org/gallery/axisartist/simple_axisline.html
https://matplotlib.org/gallery/axisartist/simple_axisline2.html
https://matplotlib.org/gallery/axisartist/demo_axisline_style.html
whose goal is just to draw a spine at x=0 or y=0.
simple_axisline2 just moves its main spine to y=0, so we can implement
that directly with Spine.set_position (see also simple_axisartist1).
simple_axisline adds additional spines, and I added an equivalent
implementation using secondary_axis, which is fairly transparent.
(This example, as well as test_secondary_xy, show why the axes patch
must be made invisible: otherwise, the patch of later secondary axes
would be drawn over other secondary axes.)
demo_axisline_style doesn't showcase anything that's specifically
related to SubplotZero so I just rewrote it without SubplotZero.
If we agree that secondary_axis is a suitable replacement, SubplotZero
could ultimately be deprecated (in a later PR).
Minor points:
Also delete
SecondaryAxis._loc
, which is set in a single place andnever used.
PR Summary
PR Checklist