Skip to content

Fix suptitle out of layout #19805

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 30, 2021
Merged

Fix suptitle out of layout #19805

merged 1 commit into from
Mar 30, 2021

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Mar 28, 2021 •
edited
Loading

PR Summary

Closes: #19803

Subtitles should not be in constrained layout if they were positioned manually because constrained layout moves them. However, in 3.4 we took them out of the layout altogether, which breaks bbox_inches='tight' and the inline backend.

Have a check for the hard-coded defaults. Maybe not the most robust, but we don't store the defaults anywhere. Could maybe store on figure and get from there, but...

Storing on the subtitle object itself works fine.

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@jklymak jklymak added this to the v3.4.1 milestone Mar 28, 2021
@jklymak jklymak added topic: geometry manager LayoutEngine, Constrained layout, Tight layout Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. and removed Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labels Mar 28, 2021
@jklymak jklymak force-pushed the fix-suptitle-out-of-layout branch 2 times, most recently from a8e20f4 to aef1db1 Compare March 28, 2021 06:44
@tacaswell
Copy link
Member

So if the user lets it be auto-positioned on init and then manually adjusts it we will still try to move it around with constrained layout unless they also manually opt out?

I think we could flip _auto_pos to false in set_position and then flip it back in the constrained layout code, but that seems like a lot of complexity for a special case that has an explicit opt-out!

@jklymak
Copy link
Member Author

jklymak commented Mar 28, 2021 •
edited
Loading

So if the user lets it be auto-positioned on init and then manually adjusts it we will still try to move it around with constrained layout unless they also manually opt out?

yes, that is correct. The subtitle is just a Text object - we could make it a light subclass and wrap set_xy to toggle _autopos. Not sure if its worth the effort, versus the user just changing the subtitle explicitly with suptitle()

I think we could flip _auto_pos to false in set_position and then flip it back in the constrained layout code, but that seems like a lot of complexity for a special case that has an explicit opt-out!

Right, but again, that would mean making suptitle a new class. OTOH they are getting some extra info hung off the objects, so maybe it makes sense to mage the Text subclass?

QuLogic
QuLogic reviewed Mar 29, 2021
View reviewed changes
@jklymak jklymak force-pushed the fix-suptitle-out-of-layout branch from aef1db1 to a38d94e Compare March 30, 2021 04:59
@jklymak jklymak force-pushed the fix-suptitle-out-of-layout branch from a38d94e to 6924c26 Compare March 30, 2021 14:33
@QuLogic QuLogic merged commit 67eca83 into matplotlib:master Mar 30, 2021
@meeseeksmachine meeseeksmachine mentioned this pull request Mar 30, 2021
Merged
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Mar 30, 2021
@QuLogic @meeseeksmachine
Backport PR matplotlib#19805: Fix suptitle out of layout
9fc747e
@jklymak jklymak deleted the fix-suptitle-out-of-layout branch March 30, 2021 20:28
QuLogic added a commit that referenced this pull request Mar 30, 2021
@QuLogic
Merge pull request #19827 from meeseeksmachine/auto-backport-of-pr-19…
9e4446a 
…805-on-v3.4.x

Backport PR #19805 on branch v3.4.x (Fix suptitle out of layout)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tacaswell tacaswell tacaswell approved these changes

@QuLogic QuLogic QuLogic approved these changes

Assignees
No one assigned
Labels
topic: geometry manager LayoutEngine, Constrained layout, Tight layout
Projects
None yet
Milestone
v3.4.1
Development

Successfully merging this pull request may close these issues.

Suptitle positioning messed up in 3.4.0
3 participants
@jklymak @tacaswell @QuLogic