FIX: remove redundant _make_twin_axes from _subplots by jklymak · Pull Request #21413 · matplotlib/matplotlib

jklymak · 2021-10-21T09:21:40Z

PR Summary

For some reason we defined _make_twin_axes twice, and the _subplots version doesn't track movement of the original axes.

EDIT:

OK, the _base _make_twin_axes used an axes_locator to position the twin. The _subplots version just gave the twin a subplotspec and used that for positioning.

In #21409, the user was doing

fig, ax = plt.subplots()
ax.set_position([0.5, 0.5, 1, 1])
ax2 = ax.twinx()

and the twin ax2 was put in the subplotspec position rather than the new position of the parent.

The change here is to always have the twin located by the parent, regardless of how the parent was created. I think that is correct, but I guess its debatable whether we want to change this, or if we want to just tell the user to create the original axes without the subplotspec mechanism. (ie. w/ fig.add_axes([0.5, 0.5, 1, 1]))

In either case, this isn't urgent for 3.5.2, so I've moved to 3.6

PR Checklist

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (run flake8 on changed files to check).
New features are documented, with examples if plot related.
Documentation is sphinx and numpydoc compliant (the docs should build without error).
Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

jklymak · 2022-04-01T08:43:27Z

OK, so now this fails if you try an apply tight_layout because the twin axes is locatable, and not a subplot.

One method is to make this axes a child of the parent, and then it won't appear in the axes list for the figure. That, however, comes with z-order changes. Currently ax2 is drawn after the parent, because it was added after. If we make it a child it can be drawn before some artists in the parent.

QuLogic · 2022-04-05T03:09:45Z

If we make it a child it can be drawn before some artists in the parent.

Interestingly, I think that is sometimes something people want? At least when using pickers and stuff. But it would likely be a breaking change to modify the order.

jklymak · 2022-04-05T07:43:22Z

If we make it a child it can be drawn before some artists in the parent.

Interestingly, I think that is sometimes something people want? At least when using pickers and stuff. But it would likely be a breaking change to modify the order.

For sure, I think people want more control over where the twin is relative to the parent. However, it may also break a lot of people, and I'm not clear how we deprecate it in an obvious way.

anntzer · 2022-04-08T20:09:07Z

OK, so now this fails if you try an apply tight_layout because the twin axes is locatable, and not a subplot.

Perhaps a solution would be to make tight_layout autoignore any axes that have an axes_locator set? (after all those probably have their own idea of "where they want to go".) Or a safer approach would be to add another flag "ignored_by_layout_engine" and set that flag explicitly for twins (perhaps this can be made private, as this should mostly only affect twins, which are not child axes due to historical causality).

jklymak · 2022-04-09T14:53:37Z

make tight_layout autoignore any axes that have an axes_locator

Well, that doesn't work because we allow the contents of the twin to be included in the extent of the parent. It would be nice to just make the twin a child of the parent, but again that has draw order implications.

I think that this new version works ok - it just adds an extra check and allows the child twin to take its parent subplot spec.

I feel that twins were somewhat questionably defined as peers at some point, whereas life would be easier if one was the parent and the other the child.

anntzer · 2022-04-09T20:24:45Z

lib/matplotlib/_tight_layout.py

+
        if axes_or_locator is None:
            axes_or_locator = ax
+        elif hasattr(ax, '_twinned_axes'):


The hasattr check is unnecessary, as it's a class variable.

Oh I see. Well, this fix here is therefore not very good logic. Making the twinned axes siblings is problematic for the original issue - if we move one of the twins, the other twin is supposed to come along. But that implies one of the twins is the parent and one the child.

I guess the redundant _make_twin_axes should remain for the two cases, and likely what needs to change is set_postiion to also change the position of any twins....

if we move one of the twins, the other twin is supposed to come along

But does it even make sense to call set_position on an axes that has an axes_locator set? both are basically trying to force the axes' position. Perhaps e.g. set_position could check if an axes_locator exists and ask the user to explicitly call set_axes_locator(None) first if they really want the axes to go here and nowhere else? In that case, for twin axes, the correct way to set their position would be to set the "parent" axes' position, not the child one (which seems correct?) (Yes, I know, it's not a "parent-child" relationship in the usual sense.)

Sure, OK, lets try this. If an axes has a locator, but it is a twin, see if the twin has a locator; if not, and it has a subplotspec, use the subplotspec for tight layout. This all seems a little more convoluted than it should be, but....

anntzer · 2023-06-07T09:03:41Z

@jklymak I stumbled upon the idea of turning twin axes into child axes as well, and likewise ran into the draw-ordering problem. However, I think this can be solved just be setting a high enough zorder on the twin, so that it gets drawn last? (This means one can break the zordering by inserting artists with custom zorders even higher, but I think we can live with that, or if we want to be super careful that's something we can explicitly detect and warn about during the transition period.)
Doing a warning for people looking for the twin in the figure.axes list and not finding it there is going to be trickier; we may need to decide whether that's something we can just live with.

jklymak · 2023-06-07T16:23:51Z

...or the twin could be a special child of the parent axes that always gets completely drawn either before or after the parent. That would require another list and a bit of logic to determine if it should be drawn first or second (currently I believe it's drawn second).

The original issue was that if the parent was moved and it was created with fig, ax = plt.subplots() it had a different locator method than if the parent was created via fig.add_axes. Another approach is to not allow axes with a subplot spec to be moved, or if they are moved, convert them to non subplot spec axes, either formally or by setting the subplotspec to None or something. It seems it a user manually moves an Axes with a subplotspec they no longer want it to be part of a subplotsepc.

anntzer · 2023-06-07T17:17:09Z

...or the twin could be a special child of the parent axes that always gets completely drawn either before or after the parent. That would require another list and a bit of logic to determine if it should be drawn first or second (currently I believe it's drawn second).

To be clear, (unless I got that wrong...) currently child axes are completely drawn as a single block within the parent axes children, i.e. if there's

parent axes with

line A in parent, zorder=0
child axes X, zorder=1
- line B in child, zorder=-1
- line C in child, zorder=3
line D in parent, zorder=2

then the draw order is ABCD, not BADC: the zorder of B and C are only used within the child axes and are effectively in a different scale from the zorders of A, X, and D (I think). So to achieve the desired effect we just need to make the zorder of the child-shared axes very large and all its subartists will be drawn after all (other) subartists of the parent.

jklymak · 2023-06-07T17:30:57Z

For sure I agree, zorder is one way to make the child behave. OTOH, I'm suggesting maybe we want to have child axes or some twin axes in a different list just to remove the ambiguity with normal Artist zorder.

anntzer · 2023-06-07T18:00:03Z

I'm suggesting maybe we want to have child axes or some twin axes in a different list just to remove the ambiguity with normal Artist zorder.

I'm not sure that really helps here? (and anyways that could be added later I guess)
Now that I look at this I see that inset_axes are simply created with zorder=5, I guess that would also work, in practice, for shared axes?

jklymak marked this pull request as draft October 21, 2021 09:22

jklymak added the topic: axes label Oct 21, 2021

jklymak added this to the v3.5.1 milestone Oct 21, 2021

jklymak added status: needs tests and removed status: needs tests labels Oct 21, 2021

jklymak marked this pull request as ready for review October 21, 2021 09:49

jklymak marked this pull request as draft October 21, 2021 12:57

QuLogic modified the milestones: v3.5.1, v3.5.2 Dec 10, 2021

QuLogic added the status: needs rebase label Mar 29, 2022

jklymak force-pushed the fix-twin-axes branch from 8cbd482 to 822f746 Compare March 31, 2022 20:15

jklymak modified the milestones: v3.5.2, v3.6.0 Apr 1, 2022

jklymak removed the status: needs rebase label Apr 1, 2022

FIX: remove redundant _make_twin_axes from _subplots

3cbe665

jklymak force-pushed the fix-twin-axes branch from 7a4c8bd to 3cbe665 Compare April 1, 2022 07:34

jklymak mentioned this pull request Apr 8, 2022

Also use _TransformedBoundsLocator to position twin subplots. #22805

Closed

6 tasks

FIX

50ba304

anntzer reviewed Apr 9, 2022

View reviewed changes

jklymak mentioned this pull request Apr 10, 2022

FIX: move twins when moving parents #22817

Draft

6 tasks

FIX: get twin

a127886

QuLogic modified the milestones: v3.6.0, v3.7.0 Jul 5, 2022

github-actions bot added the status: needs rebase label Oct 20, 2022

jklymak removed this from the v3.7.0 milestone Dec 15, 2022

jklymak added this to the future releases milestone Dec 15, 2022

maf2310 mentioned this pull request Mar 13, 2026

[Bug]: twinx and twiny ignores previous set_position #21409

Open

Uh oh!

Conversation

jklymak commented Oct 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

EDIT:

PR Checklist

Uh oh!

jklymak commented Apr 1, 2022

Uh oh!

QuLogic commented Apr 5, 2022

Uh oh!

jklymak commented Apr 5, 2022

Uh oh!

anntzer commented Apr 8, 2022

Uh oh!

jklymak commented Apr 9, 2022

Uh oh!

anntzer Apr 9, 2022

Choose a reason for hiding this comment

Uh oh!

jklymak Apr 10, 2022

Choose a reason for hiding this comment

Uh oh!

anntzer Apr 10, 2022

Choose a reason for hiding this comment

Uh oh!

jklymak Apr 10, 2022

Choose a reason for hiding this comment

Uh oh!

anntzer commented Jun 7, 2023

Uh oh!

jklymak commented Jun 7, 2023

Uh oh!

anntzer commented Jun 7, 2023

Uh oh!

jklymak commented Jun 7, 2023

Uh oh!

anntzer commented Jun 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jklymak commented Oct 21, 2021 •

edited

Loading