Repository navigation
FIX: CL avoid fully collapsed axes - #11627
tacaswell merged 1 commit into
Conversation
Sorry, something went wrong.
|
This is minor, and a safety catch, so marking RC for 3.0.... |
Sorry, something went wrong.
|
Would it make sense, and be reasonably easy, to disable the bbox and constraint calculations when zoom-to-rect or zoom/pan are on? |
Sorry, something went wrong.
|
Yeah if there is a method to detect that’s what the redraw is for, I think that’d be the safest thing most of the time. Is it easy to know that the redraw was triggered for that reason? EDIT: done - easier than I though... |
Sorry, something went wrong.
| fig._layoutbox.constrained_layout_called += 1 | ||
| fig._layoutbox.update_variables() | ||
| # Now set the position of the axes... | ||
| # check if layout worked. If not, don't change positions: |
There was a problem hiding this comment.
You could move this whole check to a local function layout_is_valid(fig) and then just:
if layout_is_valid(fig):
# Now set the position of the axes...
...
or even with a second function apply_layout(fig)
if layout_is_valid(fig):
apply_layout(fig):
else:
warnings.warn(...)
I think this would help keeping an overview what's going on.
Sorry, something went wrong.
There was a problem hiding this comment.
I guess this is a taste thing. 6 lines is kind of under my "move to its own function" threshold. When debugging I find it pretty annoying to have to chase down chains of three-line rabbit holes that do something trivial.
Sorry, something went wrong.
There was a problem hiding this comment.
You are height that this is a matter of taste
My rule of thumb is: A couple of lines that have a distinguished purpose usually make up a good function. Even more so, if I need a comment to explain what they are doing. The function name can often replace the comment.
Sorry, something went wrong.
There was a problem hiding this comment.
Well thats fair 😉...
Sorry, something went wrong.
There was a problem hiding this comment.
... done - agree that is a bit nicer...
Sorry, something went wrong.
There was a problem hiding this comment.
😄 👍
Sorry, something went wrong.
PR Summary
Applying zoom to an axes with constrained_layout caused problems because the bounding box for a line on the axes was not correctly calculated when constrained_layout needed it to be. That could lead to a collapsed axes, and singular matrices for other transformations.
This PR doesn't fix the underlying problem with the zoom not setting the transform quick enough, but since layout probably doesn't need to change much under zoom, that's likely OK. However, it does stop constrained layout from being applied if any of the axes have zero width or height.
Note this was not a problem before #10682, because now ax.get_tightbbox checks all the artists in the axes.
The only drawback I can see is if someone deliberately had a zero-width/height axes on their figure. But I don't think constrained_layout would work well in that case anyways.
UPDATE: Per @efiring suggestion, I've also turned constrained_layout off if 'PAN' or 'ZOOM' are active. I think this is much safer.
PR Checklist