Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
tight_layout support for subfigure#26108
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
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Strongly suggest using layout='constrained' for similar figure layouts. |
Yes, I know the constrained layout works fine with subfigures. But for my "real" use case (which requires different subfigures have some of their subplotpars synced so that their axes align), I found it was more straight forward to to call tight_layout and then to tweak subplotpars. If this is something we can already do with the constrained layout, I will be happy to learn how. |
rcomer commentedJun 12, 2023 • 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.
You can trigger a draw and then turn constrained layout off if that helps? |
And is there a way to align axes in different subfigures after that? With tight_layout, I run tight_layout for each of subfigures (they are set to have independent subplotpar attributes), take the maximum value of By the way, independent of this can be done with the constrained layout, I think it is worthwhile to make TightLayoutEngine to support subfigures, given the fixes are rather straightforward. |
It would be easier to see what you are asking with an example. There is definitely no harm in making tight_layout work with subfigures if it doesn't require a lot of new knobs. |
| @@ -2219,6 +2219,12 @@ def __init__(self, parent, subplotspec, *, | |||
| self._set_artist_props(self.patch) | |||
| self.patch.set_antialiased(False) | |||
| def get_size_inches(self): | |||
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.
This needs to be added to thefigure.pyi file.
How odd do we think that it is forSubFigure to haveget_size_inches but notset_size_inches? I'm leaning towards that it is OK, with a small thought of "we should implementset_size_inches that raises and says you have to go talk to the (grand-)parent figure.
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 actually think that this should be private (and have a similar private method for the top level Figure that the publicget_size_inches calls, and then uses where we need the subfigure size can call the private method instead.
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.
Ok, I will introduce an private method (so noSubFigure.get_size_inches).@tacaswell : Do you think we still need to consider some sort ofset_size_inches method (likely private) that shows a reasonable error message?
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.
If the method is private, I do not have concerns about the missing setter.
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 also needs a test. I'm still not entirely clear what you are proposing the public API for this is? just attaching an engine to a single subfigure and running tight_layout on the subfigure?
| @@ -2219,6 +2219,12 @@ def __init__(self, parent, subplotspec, *, | |||
| self._set_artist_props(self.patch) | |||
| self.patch.set_antialiased(False) | |||
| def get_size_inches(self): | |||
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 actually think that this should be private (and have a similar private method for the top level Figure that the publicget_size_inches calls, and then uses where we need the subfigure size can call the private method instead.
For the public API, I am open to any suggestions. I personally think it is not necessary to introduce new public api. I don't expect I will push some more changes, including tests. |
@leejjoon did you want to push forward with this? For your use case, as I understand it, I would personally juts keep the axes in the same layout versus having them in subfigures. The idea of subfigures is that the layouts between the subfigures are independent. Of course if you are after a very specific layout - say with a larger column separation between columns of axes, then maybe subfigures+subplotpars is a reasonable way to go. Though even for that use case, I'd probably use width_ratios and blank subplots:
However, I appreciate that there are many other reasons you may want to use subfigures. |
PR summary
TightLayoutEnginefrommatplotlib.layout_enginefails when called with a subfigure. The PR is an attempt to make it to work with subfigures.Note that, unlike
Figure,Subfigureclass does not have a method oftight_layout. So, we need to manually create an instance ofTightLayoutEngineand call it with a subfigure. Here is a simple example.PR checklist