Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
lib/matplotlib/tight_layout.py Outdated
h_axes = (1 - margin_right - margin_left - hspace * (cols - 1)) / cols | ||
kwargs["wspace"] = hspace / h_axes | ||
if h_axes < 0.: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
0 without dot
fbe12fa
to6e08b02
Comparelib/matplotlib/tight_layout.py Outdated
if v_axes < 0: | ||
warnings.warn('tight_layout cannot make axes height small enough ' | ||
'to accomodate all axes decorations') | ||
kwargs["hspace"] = 0.01 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks! Fixed.
Is there a way to test that the axis is not flipped? May check that everything in |
👍 to behavior change. |
@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. |
There was a problem hiding this 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.
6e08b02
to4730719
CompareIt seems that somehow we have introduced a code-cov test drop... |
assertlen(w)==1 | ||
deftest_big_decorators_verticaltal(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
verticaltal
?
deftest_big_decorators_horizontal(): | ||
"Test that warning emited when xlabel too big" |
There was a problem hiding this comment.
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)
assertlen(w)==1 | ||
deftest_big_decorators_horizontal(): |
There was a problem hiding this comment.
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.
assertlen(w)==1 | ||
deftest_big_decorators_vertical(): |
There was a problem hiding this comment.
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.
lib/matplotlib/tight_layout.py Outdated
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. ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
accommodate (and below)
4730719
toabf260f
Compare@meeseeksdev backport to v2.2.x |
…n-v2.2.xBackport PR#10915 on branch v2.2.x
Matplotlib emits errors like the following under specific canvaslayouts:```consoleTraceback (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
Matplotlib emits errors like the following under specific canvaslayouts:```consoleTraceback (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
*Fix#35 - "bottom cannot be >= top" matplotlib errorMatplotlib emits errors like the following under specific canvaslayouts:```consoleTraceback (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 callCo-authored-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
*Fix#35 - "bottom cannot be >= top" matplotlib errorMatplotlib emits errors like the following under specific canvaslayouts:```consoleTraceback (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 callCo-authored-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Uh oh!
There was an error while loading.Please reload this page.
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.For the second and third plots, a warning is emitted...
PR Checklist