Skip to content

FIX: tight_layout having negative width axes #10915

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 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 2 commits into from
Apr 7, 2018

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Mar 29, 2018

PR Summary

Closes #4413 #8062

fig.tight_layout() could cause axes to flip sign if the title was too big, and cause the whole thing to Error if the title was really big. This PR adds a check that the axes width and height is still greater than zero after the margins have been set.

It would be nice to just collapse the axes when this happens, but thats actually a bit of work to fake because subplots_adjust 's space parameter is specified as a fraction of average axes width (not the way I'd have expressed this parameter, but...)

Discussion in #4413 suggested that something could be done to ignore the title, but otherwise compute the tight_layout. I think that's theoretically possible, but if someone has a really long title they should probably shorten it or adjust the spacing manually rather than trying to make tight_layout do something magical.

import matplotlib.pyplot as plt

fig, axs = plt.subplots(1, 2, figsize=(6,2))
leng = 95
axs[0].set_title('A'+('a' * leng))
axs[0].set_yticks([])
axs[1].set_title('B' +('b' * leng))
axs[1].set_yticks([])
axs[0].set_xlabel('title length %d' % leng)
fig.tight_layout()
plt.show()

t45
t46
t95

For the second and third plots, a warning is emitted...

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant

@jklymak jklymak added this to the v3.0 milestone Mar 29, 2018
h_axes = (1 - margin_right - margin_left - hspace * (cols - 1)) / cols
kwargs["wspace"] = hspace / h_axes
if h_axes < 0.:
Copy link
Contributor

Choose a reason for hiding this comment

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

0 without dot

if v_axes < 0:
warnings.warn('tight_layout cannot make axes height small enough '
'to accomodate all axes decorations')
kwargs["hspace"] = 0.01
Copy link
Member

Choose a reason for hiding this comment

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

This and line 200 above fall back to different numbers, which was intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Fixed.

@tacaswell
Copy link
Member

Is there a way to test that the axis is not flipped? May check that everything in transAxes is positive?

@tacaswell
Copy link
Member

👍 to behavior change.

@jklymak
Copy link
Member Author

jklymak commented Mar 31, 2018

@tacaswell for the test given? I’ve looked at the result (as above) and it’s definitely not flipped. It’s hard to see how someone could change the code to make it flip in the test and keep the warning.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Test is good enough for me.

@jklymak jklymak force-pushed the fix-tight-layout-too-small branch from 6e08b02 to 4730719 Compare April 1, 2018 04:00
@jklymak
Copy link
Member Author

jklymak commented Apr 1, 2018

It seems that somehow we have introduced a code-cov test drop...

assert len(w) == 1


def test_big_decorators_verticaltal():
Copy link
Member

Choose a reason for hiding this comment

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

verticaltal?



def test_big_decorators_horizontal():
"Test that warning emited when xlabel too big"
Copy link
Member

Choose a reason for hiding this comment

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

emitted (and in other places)

assert len(w) == 1


def test_big_decorators_horizontal():
Copy link
Member

Choose a reason for hiding this comment

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

codecov drop is because this test name is the same.

assert len(w) == 1


def test_big_decorators_vertical():
Copy link
Member

Choose a reason for hiding this comment

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

Probably the same name as before too if you fix the other's typo.

margin_left = 0.4999
margin_right = 0.4999
warnings.warn('The left and right margins cannot be made large '
'enough to accomodate all axes decorations. ')
Copy link
Member

Choose a reason for hiding this comment

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

accommodate (and below)

@jklymak jklymak force-pushed the fix-tight-layout-too-small branch from 4730719 to abf260f Compare April 1, 2018 15:00
@efiring efiring merged commit e2a0813 into matplotlib:master Apr 7, 2018
@jklymak jklymak deleted the fix-tight-layout-too-small branch April 9, 2018 17:11
@jklymak
Copy link
Member Author

jklymak commented Jul 8, 2018

@meeseeksdev backport to v2.2.x

tacaswell added a commit that referenced this pull request Jul 8, 2018
bergercookie added a commit to bergercookie/rqt_plot that referenced this pull request Apr 3, 2020
Matplotlib emits errors like the following under specific canvas
layouts:

```console
Traceback (most recent call last):
  File "/opt/ros/kinetic/lib/python2.7/dist-packages/rqt_plot/data_plot/mat_data_plot.py", line 107, in resizeEvent
    self.figure.tight_layout()
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 1756, in tight_layout
    self.subplots_adjust(**kwargs)
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 1612, in subplots_adjust
    self.subplotpars.update(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 230, in update
    raise ValueError('bottom cannot be >= top')
ValueError: bottom cannot be >= top
```

Current patch catches and suppresses that exception.

References:
- matplotlib/matplotlib#10915
- ros-visualization#35
bergercookie added a commit to bergercookie/rqt_plot that referenced this pull request Apr 3, 2020
Matplotlib emits errors like the following under specific canvas
layouts:

```console
Traceback (most recent call last):
  File "/opt/ros/kinetic/lib/python2.7/dist-packages/rqt_plot/data_plot/mat_data_plot.py", line 107, in resizeEvent
    self.figure.tight_layout()
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 1756, in tight_layout
    self.subplots_adjust(**kwargs)
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 1612, in subplots_adjust
    self.subplotpars.update(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 230, in update
    raise ValueError('bottom cannot be >= top')
ValueError: bottom cannot be >= top
```

Current patch catches and suppresses that exception.

References:
- matplotlib/matplotlib#10915
- ros-visualization#35
dirk-thomas added a commit to ros-visualization/rqt_plot that referenced this pull request Apr 3, 2020
* Fix #35 - "bottom cannot be >= top" matplotlib error

Matplotlib emits errors like the following under specific canvas
layouts:

```console
Traceback (most recent call last):
  File "/opt/ros/kinetic/lib/python2.7/dist-packages/rqt_plot/data_plot/mat_data_plot.py", line 107, in resizeEvent
    self.figure.tight_layout()
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 1756, in tight_layout
    self.subplots_adjust(**kwargs)
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 1612, in subplots_adjust
    self.subplotpars.update(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 230, in update
    raise ValueError('bottom cannot be >= top')
ValueError: bottom cannot be >= top
```

Current patch catches and suppresses that exception.

References:
- matplotlib/matplotlib#10915
- #35

* renamed to `safe_tight_layout`, inverted logic to avoid duplicating the call

Co-authored-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
dirk-thomas added a commit to ros-visualization/rqt_plot that referenced this pull request Apr 3, 2020
* Fix #35 - "bottom cannot be >= top" matplotlib error

Matplotlib emits errors like the following under specific canvas
layouts:

```console
Traceback (most recent call last):
  File "/opt/ros/kinetic/lib/python2.7/dist-packages/rqt_plot/data_plot/mat_data_plot.py", line 107, in resizeEvent
    self.figure.tight_layout()
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 1756, in tight_layout
    self.subplots_adjust(**kwargs)
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 1612, in subplots_adjust
    self.subplotpars.update(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/matplotlib/figure.py", line 230, in update
    raise ValueError('bottom cannot be >= top')
ValueError: bottom cannot be >= top
```

Current patch catches and suppresses that exception.

References:
- matplotlib/matplotlib#10915
- #35

* renamed to `safe_tight_layout`, inverted logic to avoid duplicating the call

Co-authored-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Long axis title alters xaxis length and direction with plt.tight_layout()
7 participants