Skip to content

FIX: long titles x/ylabel layout #17222

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

Merged
merged 1 commit into from
Apr 30, 2020
Merged

FIX: long titles x/ylabel layout #17222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion doc/api/api_changes_3.3/behaviour.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ no longer accept the unsupported ``'best'`` location. Previously, invalid Axes
locations would use ``'best'`` and invalid Figure locations would used ``'upper
right'``.


Passing Line2D's *drawstyle* together with *linestyle* is removed
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand All @@ -214,3 +213,18 @@ Upper case color strings

Support for passing single-letter colors (one of "rgbcmykw") as UPPERCASE
characters is removed; these colors are now case-sensitive (lowercase).

tight/constrained_layout no longer worry about titles that are too wide
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

*tight_layout* and *constrained_layout* shrink axes to accommodate
"decorations" on the axes. However, if an xlabel or title is too long in the
x direction, making the axes smaller in the x-direction doesn't help. The
behavior of both has been changed to ignore the width of the title and
xlabel and the height of the ylabel in the layout logic.

This also means there is a new keyword argument for `.axes.Axes.get_tightbbox`:
``for_layout_only``, which defaults to *False*, but if *True* returns a
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'.
6 changes: 5 additions & 1 deletion lib/matplotlib/_constrained_layout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,11 @@ def _make_layout_margins(ax, renderer, h_pad, w_pad):
fig = ax.figure
invTransFig = fig.transFigure.inverted().transform_bbox
pos = ax.get_position(original=True)
tightbbox = ax.get_tightbbox(renderer=renderer)
try:
tightbbox = ax.get_tightbbox(renderer=renderer, for_layout_only=True)
except TypeError:
tightbbox = ax.get_tightbbox(renderer=renderer)

if tightbbox is None:
bbox = pos
else:
Expand Down
36 changes: 29 additions & 7 deletions lib/matplotlib/axes/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4086,11 +4086,15 @@ def get_default_bbox_extra_artists(self):
for _axis in self._get_axis_list():
artists.remove(_axis)

artists.remove(self.title)
artists.remove(self._left_title)
artists.remove(self._right_title)

return [artist for artist in artists
if (artist.get_visible() and artist.get_in_layout())]

def get_tightbbox(self, renderer, call_axes_locator=True,
bbox_extra_artists=None):
bbox_extra_artists=None, *, for_layout_only=False):
"""
Return the tight bounding box of the axes, including axis and their
decorators (xlabel, title, etc).
Expand All @@ -4116,6 +4120,10 @@ def get_tightbbox(self, renderer, call_axes_locator=True,
caller is only interested in the relative size of the tightbbox
compared to the axes bbox.

for_layout_only : default: False
The bounding box will *not* include the x-extent of the title and
the xlabel, or the y-extent of the ylabel.

Returns
-------
`.BboxBase`
Expand All @@ -4141,22 +4149,37 @@ def get_tightbbox(self, renderer, call_axes_locator=True,
self.apply_aspect()

if self.axison:
bb_xaxis = self.xaxis.get_tightbbox(renderer)
igl = 'x' if for_layout_only else None
try:
bb_xaxis = self.xaxis.get_tightbbox(renderer, ignore_label=igl)
except TypeError:
# in case downstream library has redefined axis:
bb_xaxis = self.xaxis.get_tightbbox(renderer)
if bb_xaxis:
bb.append(bb_xaxis)

bb_yaxis = self.yaxis.get_tightbbox(renderer)
igl = 'y' if for_layout_only else None
try:
bb_yaxis = self.yaxis.get_tightbbox(renderer, ignore_label=igl)
except TypeError:
# in case downstream library has redefined axis:
bb_xaxis = self.yaxis.get_tightbbox(renderer)
Copy link
Member

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
Member Author

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
Member Author

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
Member Author

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

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

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
Member Author

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'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.

if bb_yaxis:
bb.append(bb_yaxis)

self._update_title_position(renderer)

axbbox = self.get_window_extent(renderer)
bb.append(axbbox)

for title in [self.title, self._left_title, self._right_title]:
if title.get_visible():
bb.append(title.get_window_extent(renderer))
bt = title.get_window_extent(renderer)
if for_layout_only and bt.width > 0:
# make the title bbox 1 pixel wide so its width
# is not accounted for in bbox calculations in
# tight/constrained_layout
bt.x0 = (bt.x0 + bt.x1) / 2 - 0.5
bt.x1 = bt.x0 + 1.0
bb.append(bt)

bbox_artists = bbox_extra_artists
if bbox_artists is None:
Expand All @@ -4179,7 +4202,6 @@ def get_tightbbox(self, renderer, call_axes_locator=True,
and 0 < bbox.width < np.inf
and 0 < bbox.height < np.inf):
bb.append(bbox)

return mtransforms.Bbox.union(
[b for b in bb if b.width != 0 or b.height != 0])

Expand Down
22 changes: 20 additions & 2 deletions lib/matplotlib/axis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1079,10 +1079,15 @@ def _get_tick_bboxes(self, ticks, renderer):
[tick.label2.get_window_extent(renderer)
for tick in ticks if tick.label2.get_visible()])

def get_tightbbox(self, renderer):
def get_tightbbox(self, renderer, *, ignore_label=None):
"""
Return a bounding box that encloses the axis. It only accounts
tick labels, axis label, and offsetText.

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.
"""
if not self.get_visible():
return
Expand All @@ -1100,11 +1105,24 @@ def get_tightbbox(self, renderer):

bboxes = [
*(a.get_window_extent(renderer)
for a in [self.label, self.offsetText]
for a in [self.offsetText]
if a.get_visible()),
*ticklabelBoxes,
*ticklabelBoxes2,
]
# take care of label
if self.label.get_visible():
bb = self.label.get_window_extent(renderer)
# for constrained/tight_layout, we want to ignore the label's
# width because the adjustments they make can't be improved.
# this code collapses the relevant direction
if ignore_label == 'x' and bb.width > 0:
bb.x0 = (bb.x0 + bb.x1) / 2 - 0.5
bb.x1 = bb.x0 + 1.0
elif ignore_label == 'y' and bb.height > 0:
bb.y0 = (bb.y0 + bb.y1) / 2 - 0.5
bb.y1 = bb.y0 + 1.0
bboxes.append(bb)
bboxes = [b for b in bboxes
if 0 < b.width < np.inf and 0 < b.height < np.inf]
if bboxes:
Expand Down
Binary file modified ...atplotlib/tests/baseline_images/test_constrainedlayout/constrained_layout11.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion lib/matplotlib/tests/test_constrainedlayout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def test_constrained_layout10():
@image_comparison(['constrained_layout11.png'])
def test_constrained_layout11():
"""Test for multiple nested gridspecs"""
fig = plt.figure(constrained_layout=True, figsize=(10, 3))
fig = plt.figure(constrained_layout=True, figsize=(13, 3))
gs0 = gridspec.GridSpec(1, 2, figure=fig)
gsl = gridspec.GridSpecFromSubplotSpec(1, 2, gs0[0])
gsl0 = gridspec.GridSpecFromSubplotSpec(2, 2, gsl[1])
Expand Down
12 changes: 3 additions & 9 deletions lib/matplotlib/tests/test_tightlayout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,29 +258,23 @@ def test_empty_layout():

@pytest.mark.parametrize("label", ["xlabel", "ylabel"])
def test_verybig_decorators(label):
"""Test that warning emitted when xlabel/ylabel too big."""
"""Test that no warning emitted when xlabel/ylabel too big."""
fig, ax = plt.subplots(figsize=(3, 2))
ax.set(**{label: 'a' * 100})
with pytest.warns(UserWarning):
fig.tight_layout()


def test_big_decorators_horizontal():
"""Test that warning emitted when xlabel too big."""
"""Test that doesn't warn when xlabel too big."""
fig, axs = plt.subplots(1, 2, figsize=(3, 2))
axs[0].set_xlabel('a' * 30)
axs[1].set_xlabel('b' * 30)
with pytest.warns(UserWarning):
fig.tight_layout()


def test_big_decorators_vertical():
"""Test that warning emitted when xlabel too big."""
"""Test that doesn't warn when ylabel too big."""
fig, axs = plt.subplots(2, 1, figsize=(3, 2))
axs[0].set_ylabel('a' * 20)
axs[1].set_ylabel('b' * 20)
with pytest.warns(UserWarning):
fig.tight_layout()


def test_badsubplotgrid():
Expand Down
11 changes: 9 additions & 2 deletions lib/matplotlib/tight_layout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,15 @@ def auto_adjust_subplotpars(
if all(not ax.get_visible() for ax in subplots):
continue

tight_bbox_raw = Bbox.union([
ax.get_tightbbox(renderer) for ax in subplots if ax.get_visible()])
bb = []
for ax in subplots:
if ax.get_visible():
try:
bb += [ax.get_tightbbox(renderer, for_layout_only=True)]
except TypeError:
bb += [ax.get_tightbbox(renderer)]

tight_bbox_raw = Bbox.union(bb)
tight_bbox = TransformedBbox(tight_bbox_raw,
fig.transFigure.inverted())

Expand Down