Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

anntzer
Copy link
Contributor

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 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).

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 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

@jklymak
Copy link
Member

First, I'd like to see gridspecs as always attached to figures as a logical container for the subplotspecs and their axes.constrained_layout relies on this logical nesting, and while I understand why GridSpec was made more abstract, I don't think it is of practical use to share a gridspec between figures.

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 thatsubplot2grid respects this logic at all, but the point is to properly contain child axes in the same parent so layout can be done.

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.

@anntzer
Copy link
ContributorAuthor

First, I'd like to see gridspecs as always attached to figures as a logical container for the subplotspecs and their axes. constrained_layout relies on this logical nesting, and while I understand why GridSpec was made more abstract, I don't think it is of practical use to share a gridspec between figures.

Especially now that there isFigure.add_gridspec, we're in agreement here.

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?

Initially I called the method add() or add_self(), but that seemed a bit obscure. No strong opinion on the specific naming.

I'd be pretty against [the subplot2grid replacement suggestion].

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

gs = plt.add_gridspec(3, 3)gs[0, 0].add_subplot()gs[1:, 1:].add_subplot()

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...).

@anntzeranntzerforce-pushed thesubplotspec.add_subplot branch fromeb3dd25 to34e84e1CompareJanuary 24, 2019 20:44
@jklymak
Copy link
Member

Don’t we haveplt.subplot, which I assume can take a subplotspec as an argument? If not, that’s the pyplot change I’d make. I.e

gs = plt.add_gridspec(3, 3)plt.subplot(gs[0, 0])plt.subplot(gs[1:, 1:])

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 forplt.subplots,plt.figure and maybeplt.show().

@anntzer
Copy link
ContributorAuthor

plt.subplot() already can already take a subplotspec indeed. So perhaps just addadd_gridspec to pyplot?
I share your little love for pyplot, but see e.g.#12513 for another PoV...

@jklymak
Copy link
Member

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 readAxes andFigure docs in the API guide and get to most of the methods. Here they'd have to get toSubplotSpec, which is something that is only made by aGridSpec call. Thats a pretty deep search to find what we think is the canonical way to add a new subplot to a figure.

@jklymak
Copy link
Member

Ooops:

So perhaps just addadd_gridspec to pyplot?

Could do - I think pyplot tries to make the user ignorant of objects, whereasadd_gridspec returns objects, so its mixing paradigms?

@anntzer
Copy link
ContributorAuthor

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 actuallynot documented in the API that add_subplot() can take a gridspec as argument (#12114).
So from a discoverable documentation point of view, one should add a description of GridSpecs to add_subplot... but then one may just as well point to GridSpec.add_subplot() (or however we want to name it).

Also, I would say that an API that's like

gs = fig.add_gridspec(...)fig.add_subplot(gs[...])

immediately makes me wonder why you need to specifyfig twice, and whethergs[...] can be used on another figure (but perhaps that's just because I look at APIs in a strange way...).

@jklymak
Copy link
Member

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 encouragefig.add_subplot as the primary way that happens.

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.

Copy link
Member

@jklymakjklymak left a comment
edited
Loading

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.

@anntzer
Copy link
ContributorAuthor

If the general agreement is to reject this API, that's fine with me too... just waiting for comments from others :)

@jklymak
Copy link
Member

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.

@anntzeranntzerforce-pushed thesubplotspec.add_subplot branch from34e84e1 to164d0bbCompareFebruary 2, 2019 18:19
@anntzer
Copy link
ContributorAuthor

Reverted the examples.

Now that the preferred(?) method of creating GridSpecs attaches theGridSpec to a given figure (figure.add_gridspec), calling`fig.add_subplot(gs[...])` effectively specifies the figure informationtwice.  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 somewhatunwieldy 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).
@anntzeranntzerforce-pushed thesubplotspec.add_subplot branch from164d0bb to15e78aeCompareFebruary 2, 2019 21:11
@jklymak
Copy link
Member

Ha, well, you didn't have to revert themall! I'll approve, but the docs also have to build...

@jklymakjklymak added this to thev3.2.0 milestoneFeb 7, 2019
@ImportanceOfBeingErnest
Copy link
Member

I find this API a bit obscure. Logically

object.add_something(where)

makes much more sense than

where.add_something()

@anntzer
Copy link
ContributorAuthor

I suggested add() or add_self() above too; not sure it's better.

@ImportanceOfBeingErnest
Copy link
Member

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:

gs = gridspec.GridSpec(2,2)gs[0,1].add_subplot()

So one should probably try to avoid tricking users into attempting that by providing such method.

@anntzer
Copy link
ContributorAuthor

I think(?) we now encourage to build gridspecs usingfig.add_gridspec rather than invoking the GridSpec constructor directly, which is the case this API supports.

@ImportanceOfBeingErnest
Copy link
Member

So either you have a figure handle as in

gs = fig.add_gridspec(2, 2)
fig.add_subplot(gs[0, 0])

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.

@anntzer
Copy link
ContributorAuthor

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 lookupax.figure to get the owning figure. For example you can write_, ax = plt.subplots() and not even have afig variable around.

If you have multiple figures and gridspecs, likewise you(I)'d like to not keep track of both figures and gridspecs (gs = plt.figure().add_gridspec(...) without even bothering with afig variable). Obviously one could writegs.figure.add_subplot(gs[...]), butgs[...].add_subplot() just seems much less redundant.

@timhoffm
Copy link
Member

I'm -0.5 on this.

  1. I've an odd feeling on adding yet another way to add a subplot.

  2. The canonical use case

gs = fig.add_gridspec(2, 2)fig.add_subplot(gs[0, 0])

is already straight forward and doesn't benefit from the proposed

gs = fig.add_gridspec(2, 2)gs[0, 0].add_subplot()

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.

  1. IMO the dynamic case with multiple figures is rare. In itself it does not justify adding additional API. Additionally,gs.figure.add_subplot(gs[...]) is a bit verbose, but also more explicit thangs[...].add_subplot().

  2. pyplot is not a strong argument either. I don't think it needs to expose the full API and is mostly used for simpler plots. Nevertheless you could

gs = plt.add_gridspec(3, 3)plt.add_subplot(gs[0, 0])plt.add_subplot(gs[1:, 1:])

working ongcf() which is even more in the spirit of "no objects" compared togs[0, 0].add_subplot().

@anntzer
Copy link
ContributorAuthor

Let's just close this.

@anntzeranntzer deleted the subplotspec.add_subplot branchMarch 17, 2019 16:27
@anntzeranntzer mentioned this pull requestMay 7, 2019
@anntzeranntzer mentioned this pull requestNov 30, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jklymakjklymakjklymak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.2.0
Development

Successfully merging this pull request may close these issues.

4 participants
@anntzer@jklymak@ImportanceOfBeingErnest@timhoffm

[8]ページ先頭

©2009-2025 Movatter.jp