FIX: long titles x/ylabel layout #17222

jklymak · 2020-04-23T05:26:44Z

PR Summary

Previously, really long titles and axes labels would kill both tight_layout and constrained_layout because they vainly would try and reduce the size of the axes to somehow make the long title "fit". However, this cannot work, so instead just ignore the dimensions these can get too long in.

This leads to a few changes, but I still think is better than squishing the axes.

import matplotlib.pyplot as plt

fig, axs = plt.subplots(1, 2, constrained_layout=True)
text = 'My Really long egregious, silly title that should have a carriage return'
for ax in axs:
    ax.set_title(text)

plt.show()

Before:

testLongTitle.py:12: UserWarning: constrained_layout not applied.  At least one axes collapsed to zero width or height.

(tight layout had a similar check and was not applied)

After

Now the user gets the layout manager, and the title is allowed to overflow.

The only case where maybe this is dubious is just for constrained layout

import matplotlib.pyplot as plt
fig, axs = plt.subplots(1, 2, constrained_layout=True)
text = 'My somewhat long title that overflows a bit'
for ax in [axs[0]]:
    ax.set_title(text)
plt.show()

Old:

Moved the undecorated axes over:

New

Just lets it look bad:

I think the latter is actually better because it tells the user that they really should fix their title, whereas before they got a mysterious message about the axes collapsing and the layout not being applied.

PR Checklist

Has Pytest style unit tests
Code is Flake 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

anntzer · 2020-04-23T19:53:35Z

lib/matplotlib/axes/_base.py

+                bt = title.get_window_extent(renderer)
+                if for_layout_only and bt.width > 0:
+                    bt.x0 = (bt.x0 + bt.x1) / 2 - 0.001
+                    bt.x1 = bt.x0 + 0.002


I got why you do this (get a nonzero but small width bbox so that the thing doesn't get cancelled out, centered at the right place) but it's a bit... mysterious and could perhaps use a comment. Also, I think instead of picking an arbitraryish epsilon=0.001 here (and a different one for axis labels) you can use e.g. 0.5, as texts can't be less than one pixel wide anyways.

anntzer · 2020-04-23T19:54:34Z

doc/api/api_changes_3.3/behaviour.rst

+bounding box using the rules above.  `.axis.Axis.get_tightbbox` gets an
+``ignore_label`` keyword argument, which is *None* by default, but which can
+also be 'x' or 'y'. These API changes are public, but are meant to be
+mostly used internally.


You could make the kwargs _for_layout_only/_ignore_label with a leading underscore, depending on how private you want to make them...

Yeah, thought about it, but its possible someone else would like the functionality?

I'm fine if you want to declare this public API, I'm just not a fan of "mostly private, but not really."

anntzer · 2020-04-23T19:57:52Z

lib/matplotlib/axis.py

+        If ``ignore_label`` is 'x', then the width of the label is collapsed
+        to near zero.  If 'y', then the height is collapsed to near zero. This
+        is for tight/constrained_layout to be able to ignore too-long labels
+        when doing their layout.  


Perhaps instead use the same kwarg as for Axes.get_tightbbox (to simplify documentation), i.e. for_layout_only, and then lookup self.axis_name to know which direction to ignore?

the docs are pretty sparse for axis.get_tightlayout and I wasn't sure about introspection on the axis name as a very robust way to do this.

We look up rcparams based on axis_name, so that should be safe.

... by the way, how does that interact with polar plots?

Not sure about polar plots.... Does tight_layout work with those at all?

Looks so (e.g. try subplots(2, 2, subplot_kw=dict(projection="polar")); title("foo"); tight_layout()).

OK, well that code runs fine with this PR....

tacaswell · 2020-04-24T00:25:26Z

I am 👍 on this in principle.

How terrible would it be to put the calls in try...except TypeError block to warn and fall back to the old signature if we get any Axes / Axis subclasses that do funny things here (thinking of cartopy / wcxaxes).

attn @astrofrog @QuLogic

jklymak · 2020-04-24T02:00:45Z

You meaning someone redefined get_tightbbox and were strictly checking arguments? It’s a pretty low level function to be strict with.

QuLogic · 2020-04-24T05:12:34Z

There's only one call to label.get_tightbbox(renderer) in Cartopy, and no subclassed definitions of get_tightbbox.

QuLogic · 2020-04-27T21:42:02Z

Actually, there's one PR SciTools/cartopy#1355 that would add a get_tightbbox, but it's currently out-of-date.

jklymak · 2020-04-28T15:08:48Z

Right but the problem is if they check for the argument. If they don’t it will just be ignored and they won’t get the benefit of this PR

lib/matplotlib/axes/_base.py

lib/matplotlib/_constrained_layout.py

tacaswell · 2020-04-28T15:27:57Z

lib/matplotlib/axes/_base.py

+                bb_yaxis = self.yaxis.get_tightbbox(renderer, ignore_label=igl)
+            except TypeError as e:
+                # in case downstream library has redefined axis:
+                bb_xaxis = self.yaxis.get_tightbbox(renderer)


Do we want to warn so down-stream knows they can make this signature change?

I don't know that we should warn. But now I'm wondering if passing kwords around makes sense....

An alternative is we provide new methods for these. i.e. get_tightbbox_forlayout that does the "right" thing. A second alternative is that the layout engines back out the correct info themselves, but that seems hard.

I think the way this is done now is probably still the best, but other solutions welcome.

I guess you may need something similar to #12635?

We didn't warn in #12635 so we should probably stay consistent about that.

I think since there is nothing the end user can do about the warning, and downstream libraries don't need this feature not warning is right. It may be hard to advertise for downstream libraries, but its not a super crucial feature in my opinion, just a nicety.

lib/matplotlib/_constrained_layout.py

lib/matplotlib/axes/_base.py

lib/matplotlib/tight_layout.py

jklymak added the topic: geometry manager LayoutEngine, Constrained layout, Tight layout label Apr 23, 2020

jklymak added this to the v3.4.0 milestone Apr 23, 2020

jklymak force-pushed the fix-long-titles-layout branch 3 times, most recently from 290c255 to c50f062 Compare April 23, 2020 18:27

jklymak modified the milestones: v3.4.0, v3.3.0 Apr 23, 2020

jklymak marked this pull request as ready for review April 23, 2020 19:33

jklymak force-pushed the fix-long-titles-layout branch from c50f062 to bf62cc0 Compare April 23, 2020 19:35

anntzer reviewed Apr 23, 2020

View reviewed changes

tacaswell reviewed Apr 28, 2020

View reviewed changes

lib/matplotlib/axes/_base.py Outdated Show resolved Hide resolved

jklymak force-pushed the fix-long-titles-layout branch 2 times, most recently from 36db363 to acbd1c3 Compare April 28, 2020 15:23

tacaswell reviewed Apr 28, 2020

View reviewed changes

lib/matplotlib/_constrained_layout.py Outdated Show resolved Hide resolved

tacaswell reviewed Apr 28, 2020

View reviewed changes

tacaswell approved these changes Apr 28, 2020

View reviewed changes

tacaswell reviewed Apr 30, 2020

View reviewed changes

lib/matplotlib/_constrained_layout.py Outdated Show resolved Hide resolved

QuLogic reviewed Apr 30, 2020

View reviewed changes

lib/matplotlib/axes/_base.py Outdated Show resolved Hide resolved

lib/matplotlib/axes/_base.py Outdated Show resolved Hide resolved

lib/matplotlib/tight_layout.py Outdated Show resolved Hide resolved

FIX: long titles x/ylabel layout

474a90c

jklymak force-pushed the fix-long-titles-layout branch from f5f7580 to 474a90c Compare April 30, 2020 20:31

QuLogic approved these changes Apr 30, 2020

View reviewed changes

QuLogic merged commit 67a987e into matplotlib:master Apr 30, 2020

anntzer mentioned this pull request Apr 30, 2020

Make API of get_tightbbox more consistent between Axes and Axis. #17287

Merged

6 tasks

jklymak deleted the fix-long-titles-layout branch March 10, 2024 16:25

jklymak mentioned this pull request Mar 10, 2024

[Bug]: constrained layout clips y-label above y-axis #27877

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: long titles x/ylabel layout #17222

FIX: long titles x/ylabel layout #17222

jklymak commented Apr 23, 2020 •

edited

Loading

anntzer Apr 23, 2020

anntzer Apr 23, 2020

jklymak Apr 23, 2020

anntzer Apr 23, 2020

anntzer Apr 23, 2020

jklymak Apr 23, 2020

anntzer Apr 23, 2020

jklymak Apr 23, 2020

anntzer Apr 24, 2020

jklymak Apr 24, 2020

tacaswell commented Apr 24, 2020

jklymak commented Apr 24, 2020

QuLogic commented Apr 24, 2020

QuLogic commented Apr 27, 2020

jklymak commented Apr 28, 2020

tacaswell Apr 28, 2020

jklymak Apr 28, 2020

jklymak Apr 28, 2020

jklymak Apr 28, 2020

anntzer Apr 28, 2020

tacaswell Apr 28, 2020

jklymak Apr 28, 2020

FIX: long titles x/ylabel layout #17222

FIX: long titles x/ylabel layout #17222

Conversation

jklymak commented Apr 23, 2020 • edited Loading

PR Summary

Before:

After

Old:

New

PR Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tacaswell commented Apr 24, 2020

jklymak commented Apr 24, 2020

QuLogic commented Apr 24, 2020

QuLogic commented Apr 27, 2020

jklymak commented Apr 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jklymak commented Apr 23, 2020 •

edited

Loading