Shorten tight_layout code. #17205
Shorten tight_layout code. #17205
Conversation
| vspaces = [[] for _ in range((rows + 1) * cols)] | ||
| hspaces = [[] for _ in range(rows * (cols + 1))] | ||
|
|
||
| union = Bbox.union |
There was a problem hiding this comment.
I think it was just a typing optimization :); saving an attribute access is unlikely to matter here especially given that we're about to rerender the figure after tightlayouting it...
lib/matplotlib/tight_layout.py
Outdated
| hspaces = [[] for _ in range(rows * (cols + 1))] | ||
|
|
||
| union = Bbox.union | ||
| vspaces = np.full((rows + 1, cols), None) |
There was a problem hiding this comment.
The third dimension is len(zip(subplot_list, ax_bbox_list, num1num2_list)) minus the subplots with completely invisible Axes. But since we take the sum of the lists in vspaces/hspaces, then we can just fill the empty locations with 0, and make these truly 3D?
There was a problem hiding this comment.
Actually we can just directly accumulate the values (it's not clear to me why you'd want to add paddings when multiple axes share the same subplotspec (which the comment says is possible in axes_grid?? well let's not get into that for now), but let's not change that here).
The last dim is not actyally len(zip(...)), but the max number of axes that share the same subplotspec.
ac235b2 to
dc63fdf
Compare
| margin_left = max([sum(s) for s in hspaces[::cols + 1]] + [0]) | ||
| margin_left += pad_inches / fig_width_inch | ||
|
|
||
| margin_left = (max(hspaces[:, 0].max(), 0) |
There was a problem hiding this comment.
What did the sum do? And why don‘t we need it anymore?
There was a problem hiding this comment.
For reasons I don't understand, this used to sum the margins of all subplots at a given position -- I'd have thought it's the max that matters, but let's not touch that here. (Actually I don't even know how you can realistically have multiple subplots at the same position, but there's a comment specifically mentioning this happens in axes_grid, so sure...)
Previously the code accumulated all margins in lists, and summed them at the end (hspaces[...].append(...)). Now I just make a 2D array of floats and directly sum the margins there (hspaces[...] += ...).
There was a problem hiding this comment.
Actually, looking at it again, I see at least one reason for the sum: you need to add the right margin (between the spines and the label edges) of the axes on the left and the left margin of the axes on the right.
|
Artifacts are missing, so I can't check the tight layout tutorial. |
- Turn hspaces/vspaces into 2D arrays, rather than flat lists that
need to be repeatedly accessed with flattened indices, and directly
accumulate widths/heights in there.
- Merge the num2 is None case by treating it as a slice of length 1,
which then allows just inlining _get_{left,right,top,bottom}.
- Minor refactorings.
|
sure, triggered a rebuild. |
that need to be repeatedly accessed with flattened indices.
which then allows just inlining get{left,right,top,bottom}.
PR Summary
PR Checklist