-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add SubplotSpec.add_subplot. #13280
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
Add SubplotSpec.add_subplot. #13280
Conversation
First, I'd like to see gridspecs as always attached to figures as a logical container for the subplotspecs and their axes. However, I'm -0.125 on this suggestion from a readability point of view. You are adding a subplot to the figure, so its a bit dissonant to "add_subplot" from the subplotspec. I guess you are adding and axes? As for: plt.figure().add_grispec(3, 3)[0, 0].add_subplot()
plt.figure().add_grispec(3, 3)[1:, 1:].add_subplot() I'd be pretty against this. It completely loses the relationship between the first axes and the second, and constrained_layout would not be trusted to lay them out properly. The way to do this would be gs = figure.add_gridspec(3, 3)
fig.add_subplot(gs[0, 0])
fig.add_subplot(gs[1:, 1:]) then all the axes are logical children of the same gridspec. ... not that As I've pointed out in the past, if we had this logical layout throughout (i.e. figure.subplot() respected it), then we could probably make tight_layout work without resorting to the constraint manager in constrained_layout. It'd be tricky, but I think possible. Whether its better than just using a constraint manager is another question. |
Especially now that there is
Initially I called the method add() or add_self(), but that seemed a bit obscure. No strong opinion on the specific naming.
Well, as you say it's not really worse than the situation with subplot2grid. I guess my real question is what is really the intended audience of subplot2grid, and whether
can be considered a reasonable pyplot-level replacement for it (not that I'd claim really knowing how to design MATLAB-style interfaces :p but that looks better IMO...). |
eb3dd25
to
34e84e1
Compare
Don’t we have
I personally don’t think we should tie ourselves up in knots making a Matlab-style interface, because MathWorks has been adding object-oriented features for decades now. I think its a strength that we were object oriented from the start. When I started Matplotlib, I was pretty confused by the Matlab-like interface of the tutorials, but as soon as anything advanced was done a whole new interface was used. It was really hard to tell the canonical way of doing things. If it were up to me, I’d deprecate everything except for |
|
I'd say adding this convenience method is not going to hurt anything. I am less convinced it should be the canonical way and used extensively in the docs. I think its mysterious that subplotspec.add_subplot() would add a subplot to a figure associated (way down) with that subplotspec. I think a naive user would (rightly) wonder what the heck is going on. Whereas I think the current canonical way of doing things makes sense. The new method is also almost completely undiscoverable - I think folks should be able to read |
Ooops:
Could do - I think pyplot tries to make the user ignorant of objects, whereas |
Of course, the "canonical" way may still be add_subplot(x, y, z) (assuming we're not talking about plt.subplots() use-cases), but it is actually not documented in the API that add_subplot() can take a gridspec as argument (#12114). Also, I would say that an API that's like
immediately makes me wonder why you need to specify |
I still think that the user is going to think "I want to add a subplot, how do I specify where it goes?" and we have two ways to do that, and that we should encourage I agree its redundant to specify fig twice, but I don't think that will grate on most people. Again, not saying this new convenience should not be there, I just don't agree with it as the easiest way to specify a set of gridspecs. But others may disagree. |
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.
So I approve this change, but I don't think all the examples should be changed to use this method. Of course if two other devs would like to see it go in as-is, its not a big deal.
If the general agreement is to reject this API, that's fine with me too... just waiting for comments from others :) |
Just to be clear - I don't reject the API, I just don't think its the most natural and shouldn't necessarily take over the docs. |
34e84e1
to
164d0bb
Compare
Reverted the examples. |
Now that the preferred(?) method of creating GridSpecs attaches the GridSpec to a given figure (figure.add_gridspec), calling `fig.add_subplot(gs[...])` effectively specifies the figure information twice. Hence, allow one to do ``` gs = fig.add_gridspec(2, 2) gs[0, 0].add_subplot() ``` as a synonym for ``` gs = fig.add_gridspec(2, 2) fig.add_subplot(gs[0, 0]) ``` Ideally, this should ultimately allow one to deprecate the somewhat unwieldy subplot2grid, replacing ``` plt.subplot2grid((3, 3), (0, 0)) plt.subplot2grid((3, 3), (1, 1), rowspan=2, colspan=2) ``` by ``` plt.figure().add_grispec(3, 3)[0, 0].add_subplot() plt.figure().add_grispec(3, 3)[1:, 1:].add_subplot() ``` or, after implementing a plt.add_gridspec() that operates on gcf(), ``` gs = plt.add_gridspec(3, 3) gs[0, 0].add_subplot() gs[1:, 1:].add_subplot() ``` A similar API change would be to make GridSpec.tight_layout() lose its `figure` argument (or rather, it could become optional).
164d0bb
to
15e78ae
Compare
Ha, well, you didn't have to revert them all! I'll approve, but the docs also have to build... |
I find this API a bit obscure. Logically
makes much more sense than
|
I suggested add() or add_self() above too; not sure it's better. |
Asking differently: What do we loose when keeping everything as it is? I can't think of an actual usecase of this, except the one that doesn't work:
So one should probably try to avoid tricking users into attempting that by providing such method. |
I think(?) we now encourage to build gridspecs using |
So either you have a figure handle as in gs = fig.add_gridspec(2, 2) in which case there is no need for the proposed method; or you don't have a figure handle, in which case the proposed method cannot be used either. |
I guess I should have motivated this more. The use case is when you are simultaneously managing multiple figures, each with its own gridspec and dynamically adding axes. If you keep track of the axes, you don't need to additionally keep track of the figures, because you can just lookup If you have multiple figures and gridspecs, likewise you(I)'d like to not keep track of both figures and gridspecs ( |
I'm -0.5 on this.
is already straight forward and doesn't benefit from the proposed
In contrast, it makes it more implicit, and doesn't reflect the actual object relation: The subplot belongs to the figure, not to the gridspec.
working on |
Let's just close this. |
Looking for comments about the proposed new API.
Now that the preferred(?) method of creating GridSpecs attaches the
GridSpec to a given figure (figure.add_gridspec), calling
fig.add_subplot(gs[...])
effectively specifies the figure informationtwice. Hence, allow one to do
as a synonym for
Ideally, this should ultimately allow one to deprecate the somewhat
unwieldy subplot2grid, replacing
by
or, after implementing a plt.add_gridspec() that operates on gcf(),
A similar API change would be to make GridSpec.tight_layout() lose its
figure
argument (or rather, it could become optional).PR Summary
PR Checklist