Fix removal of colorbars on nested subgridspecs. #27360
Conversation
Indiscriminately going back up to the topmost gridspec is wrong in the presence of nested gridspecs. We may want to add an API to just go up one level in gridspecs (i.e. public access to _subplot_spec) but I'd like to first consider whether we could e.g. merge GridSpecFromSubplotSpec into GridSpec with the only difference that the former's get_parent() (or another similarly named new method) returns another gridspec whereas the latter's returns a figure.
anntzer
added
topic: geometry manager
| subplotspec = gs.get_topmost_subplotspec() | ||
| except AttributeError: | ||
| # use_gridspec was False | ||
| subplotspec = self.ax.get_subplotspec().get_gridspec()._subplot_spec |
There was a problem hiding this comment.
Can you walk through how this works? Where did _subplot_spec come from and why is it defined on the parent gridspec?
There was a problem hiding this comment.
The classic (tighr_layout, pre-CL) way of positioning colorbars (assuming that the parent axes is positioned with a subplotspec) is to create a subgridspec on the parent subplotspec. For example say we had initially
A B
C D
and we choose to add a colorbar to axes D (which was previously positioned at gs = GridSpec(2, 2)[1, 1]). We create a subgridspec sgs = GridSpec(2, 2)[1, 1].subgridspec(3, 2) so that the block where we previously had D is replace by (zoomed in)
D .
D c
D .
where D is now at sgs[:, 1] and c (the colorbar) at sgs[1, 1] (and likewise for other colorbar locations).
Now if we remove the colorbar, we want to move back D to gs, which is exactly equal to sgs._subplot_spec (*); sgs itself is equal to c.get_subplotspec().get_gridspec().
The code prior to this PR did not go up one nesting layer, but rather climbed all of the nesting layers indiscriminately (get_topmost_subplotspec), which would be problematic if the whole thing way already layered into other nested subgridspecs.
(*) as noted in the message, maybe a public API for _subplot_spec would be nice, but that's another issue.
There was a problem hiding this comment.
I see - _subplot_spec is the subplot spec of the (Sub)GridSpec. Makes sense.
Indiscriminately going back up to the topmost gridspec is wrong in the presence of nested gridspecs. Closes #27329.
We may want to add an API to just go up one level in gridspecs (i.e. public access to _subplot_spec) but I'd like to first consider whether we could e.g. merge GridSpecFromSubplotSpec into GridSpec with the only difference that the former's get_parent() (or another similarly named new method) returns another gridspec whereas the latter's returns a figure.
PR summary
PR checklist