Add GridSpec.subplots()

[anntzer]

so that one can separately pass gridspec_kw directly to the gridspec
constructor.

Just the proposal at #14155; trying to get some more discussion on it by posting an actual PR :)

PR Summary

PR Checklist

• Has Pytest style unit tests
• Code is Flake 8 compliant
• New features are documented, with examples if plot related
• Documentation is sphinx and numpydoc compliant
• Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
• Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

[efiring]

Looks like a good idea; it allows for a clearer and more logical flow of user code.

[ImportanceOfBeingErnest]

This creates the awkward situation that having a gridspec object gs, you don't actually know if you can do gs.subplots() or not. Methods that only work conditionally should be avoided in my eyes.

More generally, I'm still of the opinion that gridspec is a layout submodule and hence should not carry any axes creation stuff inside. The fact that it has a .figure attribute is merely a hack to get constrained layout working.

One main problem I see is that fig.add_gridspec is totally counterintuitive. It doesn't add a gridspec to a figure, no! Instead it adds a figure to a gridspec.

I would be much more inclined to support any solution that allows subplots() to take a GridSpec instance, like

gs = GridSpec(2, 2, width_ratios=[3,1])
fig, axs = plt.subplots(gridspec_kw=gs)

(though of course the _kw part is misleading here, so any alias to ensure backcompat for that would be fine as well.)

[efiring]

Very good points, @ImportanceOfBeingErnest. Maybe plt.subplots(grid=gs). Or even allow it as a sole positional argument, e.g., plt.subplots(gs, sharex=True) etc.?

[anntzer]

The name add_gridspec was already bikeshedded in #11010; I do agree it's not great but heh...

If we really want to forget about the fact that gridspecs can be attached to a figure, I would rather spell this functionality gridspec.[add_]subplots(figure=...). Shoehorning this into plt.subplots() just gives us yet another function with an overloaded signature that is a headache to properly document.

[jklymak]

More generally, I'm still of the opinion that gridspec is a layout submodule and hence should not carry any axes creation stuff inside. The fact that it has a .figure attribute is merely a hack to get constrained layout working.

Just to push back on this a bit. I consider this a lack in the original hierarchy of organizing elements on a figure. A figure contains a gridspec, and gridpsec "subplots" contain axes or other gridspecs. Without the top-most container, its hard to do layout at the top level.

[ImportanceOfBeingErnest]

A gridspec is one of several ways to position axes inside a figure. Axes in a figure can be positionned according to zero or several gridspecs. Also, the same gridspec can be used to position axes in several figures. So, gridspec is not part of any hierarchy.
Since it's also not the only way to position axes (see e.g. the axes_grid toolkit), one should be careful to assume some dependency, which is not reflected in the codebase.
I think it would be nice to keep it as modular as possible to also allow for alternative positionning tools in the future, e.g. https://github.com/matplotlib/grid-strategy

[jklymak]

I'm saying gridspecs should be part of a hierarchy, though I agree that is not presently the case. But there is little practical use for them not having a parent container, compared to a huge advantage to being able to state what their parent container is. I think they should do more (i.e. gridspec.suptitle would be very useful and often asked for) and not just be geometric constructions.

[jklymak]

Sorry forgot about this one.

[jklymak]

In my opinion, grid specs should have a parent. That convenience far outweighs the very mild convenience of being able to treat them as abstract objects that can be shared across figures. Its not exactly hard to write gridspec(a, b) twice, and keep track of a, b instead of the "gridspec".

[timhoffm]

Not quite sure where this is leading us. Just from the name and original intent GridSpec is just an abstract specification of positions in figure space. Already adding a figure attribute is a deviation from this. Similarly, adding GridSpec.subplots() or GridSpec.parent goes beyond the original intent.

Disclaimer: I don't know in detail how tight_layout or constrained_layout work.

I agree with @jklymak that just an abstract specification is not enough for some use-cases. To a flexible layout mechanism would be need to distribute space to a hierarchy of elements (It's a hierarchy because you have elements that consist of more basic elements; e.g. a figure may contain two Axes, each axes may have ticklabels, maybe an axes-label etc.). The relation between those elements must be specified somehow, and a good established way appear to be Layout elements that are part of the hierarchy. GUI layouts in e.g. Java and Qt work that way and I assume that our layout requirements are not too different from GUIs.

That said, it would be awesome to have such a layout system. But it would need to be designed from scratch. I don't think we can morph GridSpec to such a system.

The other way round, I see the current and proposed extensions of GridSpec as an attempt to realize some capabilities of a more general layout system. But I'm afraid that they cannot live up to their expectations and that we'll end up with a more complex but still only half-baked solution.

[jklymak]

Constrained layout works for quite complex layouts just fine with the addition in 2.2 of passing a parent to the gridspec. While I guess if you were to rearchetect it from the beginning you might do it differently I’d be very surprised if you came up with something substantively different than what we have now. I personally don’t think we should let the fact that gridspec started as an abstract thing stop us from making it more concrete. The abstraction is only mildly useful whereas making it an actual container of axes is super useful.

[efiring]

@timhoffm I don't see how this PR is making anything more complicated. It is essentially factoring out the subplots functionality so that it can be used at a lower level.

[anntzer]

Pushed an improved version which also supports subgridspecs; see e.g. improvements to demo_subgridspec06 (the improvement would be even clearer if the subaxes needed to be shared).

[jklymak]

I don't like chained commands like this, so maybe the first time you do it:

gs = fig.add_gridspec(3, hspace=0)
axes = gs.subplots(shares=True, sharey=True)

and then devolve to the "short cut". With the one-liner I can't help but think that subplots is a method on fig, whereas its really a method on the GridSpec

[anntzer]

sure, either way is fine for me.

[timhoffm]

Did not have time to look into the details here, but I withdraw my original reservation that GridSpec should only be an abstract specification.

[jklymak]

I needed this today, and was surprised it wasn't merged! Can this get a second review?

[efiring]

@ImportanceOfBeingErnest, would you like to comment on this? In discussion above and in the meeting, the consensus is moving toward merging this in a week, unless you have objections.

[tacaswell]

I see the concern about letting "I have a figure" leak into GridSpec, but the benefit is also clear.

If we were doing this from scratch, we may not do it this way and it may be worth pulling an "Abstract" version of these out the top, but I don't think we should block on that.

We also briefly discussed a gs.copy_without_figure method that returns an "un-bound" copy.

[jklymak]

Or gsnew = gs.reuse(newfig)

[timhoffm]

As discussed in the call, we should have "layout" objects that know about the figure and the relation of its content. That's similar to layouts in GUI (I'm thinking particularly of Qt, but that's what I'm most familiar with). I'm not 100% sure if that's exactly what ConstrainedLayout does.

I'm wondering if we are trying to make GridSpec do too much here. Should we define such layouting concepts from the gound up? If so, it may be that we'd better make a GridLayout and keep GridSpec as an abstract concept instead of having bound and unbound GridSpecs. We would then have something like GridLayout.from_gridspec(gs) instead of gs.reuse(newfig) or similar.

This is clearly not thought out completely, but if we consider going in such a direction, IMHO it would be a mistake to adopt the changes proposed in this PR.

I'm fine if people think we should merge this anyway, but we should be aware what route we are taking here.

[anntzer]

Given that you mention Qt: I think Qt actually uses the same classes for bound and unbound layouts? e.g. if you have a QVBoxLayout, you typically want to attach it to a parent (well, call parent.setLayout) before you can do anything useful with it? I also doubt that you can reuse the same layout object on two parents? "The layout will automatically reparent the widgets (using QWidget::setParent())" per https://doc.qt.io/qt-5/layout.html#tips-for-using-layouts.

[timhoffm]

That's exactly the point. QVBoxLayout is quite unlike GridSpec: It's not an abstract specification but a non-reusable container bound to actual elements. (While you can instantiate a standalone QVBoxLayout, it's only useful when bound to a parent and when you've added widgets to the layout.)

I think we actually need some something like that. And I think it's better to write that from scratch than trying to modify GridSpec into it. As an abstract specification, GridSpec is a separate concept, which can coexist with bound layouts.

[jklymak]

What is the practical benefit of an abstract GridSpec versus a bound GridSpec? I appreciate that previously it was abstract, and then I hijacked it to be bound to make constrained_layout work. But that was really the least invasive thing I could do, versus trying to introduce a whole new bound object that did the same thing.

[efiring]

Discussion at the meeting on May 4 suggests that factoring out a gridspec_layout and deprecating the unbound gridspec might be a good move.