Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
ENH: Add gridspec method to figure, and subplotspecs#11010
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I don't feel that removing The I would not use constrained_layout by default throughout the tutorial. This makes it hard to see where it plays a role. Just as it was before, when I have a problem with the call to
If contrained layout needs to draw the figure twice it needs to take care of that by itself. It's not useful to show examples with undeterministic behaviour. Up till now you never ever needed to draw a figure before showing it in matplotlib. Introducing this practice between the lines is really 👎 . |
@ImportanceOfBeingErnest Fair enough about keeping a few
I appreciate the problem about The alternative, from |
7f4d162
toc0f64eb
CompareI'm stumbling over the name Not knowing if an additional If this cannot be solved easily, can we please use examples that do not require workaround for flaws, unless the example is explicitly designed to show the limitations and explain how to cope with them. |
I just saw thatthis example disappeared from the tutorial. This is the most useful of all to show how to use Gridspec and would really like it not to be thrown away. (I guess that happened already in a previous version, but still...) My point about the constrained_layout use is that you define it at the very beginning. Instead it should be inside the example itself. Otherwise users copy the example, run it and get a different figure. You cannot expect them to check for some setting being done at some completely different place. Personally I would prefer there to be a separete section which explains how you may use
I do think those weight strong enough to actually add this function. But I do not see that they help in understanding |
jklymak commentedApr 10, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Agreed. I contemplated adding a list of gridspecs to figure, but decided it wasn't totally necessary. But from user's point of view, they are adding a gridspec, and I'd argue the name makes sense. I could be convinced that it should be just
OK, fair enough - I'll change that. And I can add back in the complicated example. I liked that one too, and in fact prefer it to the somewhat messy double nested thing at the end of the current tutorial.
I understand what you are saying. I just disagree that a) someone wants to use the abstraction ability of having a GridSpec they drag from one figure to the next, a feature I find of dubious value. But more to the point, the new convenience methods areexactly the same as the direct method, so I don't think any understanding is lost. |
What I mean is that you'd create the gridspec once and use it for all figures you create.
|
Yeah I understand the use-case. Pretty skeptical that gets used much in practice though. If we completely got rid of that interface you’d still get the same thing by packing the args in a dict. Not that I think we should get rid of the fundamental interface at all. Would an example like the above explaining the advantage ofnot using the convenience method make sense? The big disadvantage of your example is that constrained_layout won’t work at all, whereas it works fine for the new interface. My overarching point for a while is that we have this nice hierarchical way of organizing subplots, but we don’t keep track of that hierarchy, and hence lose information about the users desires. Without that hierarchy automating layout is impossible for general layouts. This is a small step to enforcing that hierarchy at a slight loss of convenience in some obscure use cases. |
65cdd57
to6ae08fe
CompareRemoving WIP as this is the extent of what I am planning. However, open to
|
8618fda
to2d337ac
CompareI'm strongly in favor of this and I understand concerns about the naming. what about |
set_gridspec strongly suggests that a figure can only ever haveone gridspec attached at a time, which seems a bit too strong (even though "mostly" true). |
jklymak commentedApr 10, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Re name: I just think of it as parallel to
If it would make things nicer, I'dlove it if a figure kept track of it's child gridspec. But thats consistency fromour point of view. I think If I don't prevail in my argument here, |
Agreed.
|
Both If there would be an agreement that the figure should keep a reference to the gridspec (as suggested by@jklymak), it would be ok to use these terms in anticipation of the future functionality. Then again, it's probably better think of a more general API like:
So -0.5 on For now, I think |
jklymak commentedApr 11, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
OK, due to popular demand, I added |
Though I still prefer |
5e31375
to4a64f3d
CompareWhy not let the figure have a gridspec by default? |
@Tillsten A gridspec needs the grid parameters (m, n) before it means anything, so that needs user input I think... We could just put a default one in, and indeed thats what happens if there is just one axes in the figure. |
jklymak commentedApr 12, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Actually, this probably should at least look for where |
This had a lot of good input; anyone up for a full review? Its not a big conceptual change, though I did edit the tutorial a fair bit... |
Pinging for review on this. Its a pretty straightforward enhancement, and I think I addressed the concerns about the name |
e1bd2d2
tod138cda
Compare...rebased |
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.
I think this represents a good improvement in usability. Thanks for sticking with this,@jklymak
d138cda
toaec8d6a
Comparejklymak commentedJul 10, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Squash and rebase. I think it'd be wrong to say that this is release critical, but it'd be nice to include in 3.0; its pretty straightforward, just adding new access points to the gridspec w/o the user having to know all the magical incantations... |
jklymak commentedJul 10, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
EDIT: fixed now... Doc build fails w/
I forget how to fix that. Note that the |
30a8265
tob712200
CompareThere 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.
This is a useful simplification.
lib/matplotlib/gridspec.py Outdated
Other Parameters | ||
---------------- | ||
*kwargs* are passed to `.GridSpec`. |
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.
Must be formatted like parameters:
Other Parameters ---------------- **kwargs All other parameters are passed to `.GridSpec`.
b712200
toc366d9b
Comparec366d9b
to2977f37
CompareSelf merging. This did pass AppVeyor before the rebase, and the rebase was trivial, and had no Windows related changes. |
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
For contrained_layout, I'm trying to encourage attaching a figure to
GridSpec
s. This PR makes aadd_gridspec
method available toFigure
.Old
New
New
SubplotSpec
method:It also makes a convenience method available to
SubplotSpec
to createSubplotspecGridSpecs
Old
New
PR Checklist