NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

TightLayoutEngine.execute runs get_tight_layout_figure twice#26203

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

Jump to bottom

Open

leejjoon wants to merge1 commit intomatplotlib:main

base:main

Choose a base branch

fromleejjoon:tightlayout-twice

Open

TightLayoutEngine.execute runs get_tight_layout_figure twice#26203

leejjoon wants to merge1 commit intomatplotlib:mainfromleejjoon:tightlayout-twice

Conversation

Copy link

Contributor

leejjoon commentedJun 27, 2023

PR summary

IfAutoLocator is used, changing the location of axes may change the ticklabels.
Thus the result of tight_layout may not be optimal.

fig=plt.figure(layout="tight",figsize=(3,3))fig.subplots(3,3)fig.set_facecolor("0.8")

The PR simply runs the algorithm twice.

ConstrainedLayoutEngine also runs its algorithm twice (_constrained_layout.do_constrained_layout)
There are failing tests, because the result of running the algorithm twice can be different for running it only once. We need to update the baseline images if this PR to be accepted.
I can add a simple test if requested.
My intention is to have this PR merged before I further work ontight_layout support for subfigure #26108.

PR checklist

new and changed code istested

TightLayoutEngine.execute runs get_tight_layout_figure twice

29f7621

QuLogic added the topic: geometry managerLayoutEngine, Constrained layout, Tight layout label

Jun 27, 2023

jklymak reviewed

Nov 30, 2023

View reviewed changes

Copy link

Member

jklymak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This seems like a fine thing to do. Not sure if we want to change that many images.

rcomer mentioned this pull request

Feb 6, 2025

DOC: Document that tight_layout may not converge#29585

Merged

Copy link

Member

timhoffm commentedFeb 6, 2025

Given that most people should be using constrained_layout nowadys, and that tight_layout has other shortcomings as well, I think that changing the behavior - even if it means improving - is not worth it. Those who are using tight_layout are likely ok with the existing behavior. Those who want a better layout engine should switch to constrained_layout.

I therefore propose to close this.