Previously, really long titles and axes labels would kill bothtight_layout andconstrained_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.

importmatplotlib.pyplotaspltfig,axs=plt.subplots(1,2,constrained_layout=True)text='My Really long egregious, silly title that should have a carriage return'foraxinaxs: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

importmatplotlib.pyplotaspltfig,axs=plt.subplots(1,2,constrained_layout=True)text='My somewhat long title that overflows a bit'foraxin [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 isFlake 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

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

Apr 23, 2020

jklymak added this to thev3.4.0 milestone

Apr 23, 2020

jklymak force-pushed thefix-long-titles-layout branch 3 times, most recently from290c255 toc50f062Compare

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 thefix-long-titles-layout branch fromc50f062 tobf62cc0Compare

April 23, 2020 19:35

anntzer reviewed

Apr 23, 2020

View reviewed changes

lib/matplotlib/axes/_base.py Outdated

		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

Copy link

Contributor

anntzerApr 23, 2020

Choose a reason for hiding this comment

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

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 reviewed

Apr 23, 2020

View reviewed changes

doc/api/api_changes_3.3/behaviour.rst Outdated

		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.

Copy link

Contributor

anntzerApr 23, 2020

Choose a reason for hiding this comment

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

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

Copy link

MemberAuthor

jklymakApr 23, 2020

Choose a reason for hiding this comment

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

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

Copy link

Contributor

anntzerApr 23, 2020

Choose a reason for hiding this comment

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

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

anntzer reviewed

Apr 23, 2020

View reviewed changes

lib/matplotlib/axis.py Outdated

		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.

Copy link

Contributor

anntzerApr 23, 2020

Choose a reason for hiding this comment

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

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?

Copy link

MemberAuthor

jklymakApr 23, 2020

Choose a reason for hiding this comment

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

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

Copy link

Contributor

anntzerApr 23, 2020

Choose a reason for hiding this comment

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

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

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

Copy link

MemberAuthor

jklymakApr 23, 2020

Choose a reason for hiding this comment

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

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

Copy link

Contributor

anntzerApr 24, 2020

Choose a reason for hiding this comment

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

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

Copy link

MemberAuthor

jklymakApr 24, 2020

Choose a reason for hiding this comment

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

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

Copy link

Member

tacaswell commentedApr 24, 2020

I am 👍 on this in principle.

How terrible would it be to put the calls intry...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

Copy link

MemberAuthor

jklymak commentedApr 24, 2020

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

Copy link

Member

QuLogic commentedApr 24, 2020

There's only one call tolabel.get_tightbbox(renderer) in Cartopy, and no subclassed definitions ofget_tightbbox.

Copy link

Member

QuLogic commentedApr 27, 2020

Actually, there's one PRSciTools/cartopy#1355 that would add aget_tightbbox, but it's currently out-of-date.

Copy link

MemberAuthor

jklymak commentedApr 28, 2020

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

tacaswell reviewed

Apr 28, 2020

View reviewed changes

lib/matplotlib/axes/_base.py OutdatedShow resolvedHide resolved

jklymak force-pushed thefix-long-titles-layout branch 2 times, most recently from36db363 toacbd1c3Compare

April 28, 2020 15:23

tacaswell reviewed

Apr 28, 2020

View reviewed changes

lib/matplotlib/_constrained_layout.py OutdatedShow resolvedHide resolved

tacaswell reviewed

Apr 28, 2020

View reviewed changes

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)

Copy link

Member

tacaswellApr 28, 2020

Choose a reason for hiding this comment

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

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

Copy link

MemberAuthor

jklymakApr 28, 2020

Choose a reason for hiding this comment

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

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

Copy link

MemberAuthor

jklymakApr 28, 2020

Choose a reason for hiding this comment

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

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.

Copy link

MemberAuthor

jklymakApr 28, 2020

Choose a reason for hiding this comment

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

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

Copy link

Contributor

anntzerApr 28, 2020

Choose a reason for hiding this comment

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

I guess you may need something similar to#12635?

Copy link

Member

tacaswellApr 28, 2020

Choose a reason for hiding this comment

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

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

Copy link

MemberAuthor

jklymakApr 28, 2020

Choose a reason for hiding this comment

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

I think since there is nothing the end user can do about the warning, and downstream libraries don'tneed 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.