Fix constrained layout applying pad multiple times#30108
Conversation
|
This seems OK. However, can we make sure it works when there actually is a colorbar? ( |
|
Rebased and got linting failures I don't understand. They are about rst files. |
|
Does this need a test for a case with a colorbar, and if so is an image comparison test based on #30108 (comment) enough? |
|
I think so. I may have limited availability but feel free to ping for a review after 13 may. Or other folks can merge. I think this approach is good |
QuLogic
left a comment
There was a problem hiding this comment.
The change seems reasonable and the updated figures don't look broken.
|
@QuLogic can tihs still go into 3.11? Because it's a visual change, do we want to mention it in the release notes? |
|
I think we may still have some room for it. |
|
I can write a release note if desired. Should I put it in the usual place in this PR? Or at this point is it simpler to just provide some text to go straight into #31642? |
|
Added the changenote here as I figured I needed to check it renders OK. Note that given import matplotlib.pyplot as plt
plt.subplot_mosaic('AC;BC', layout='constrained')
plt.savefig('simple_mosaic.png')
plt.subplots(2, 2, layout='constrained')
plt.savefig('simple_subplots.png')This PR make the spacing for the left column the same in both figures. So I think the smaller spacing can be considered a bug fix. Of course it may still be surprising to some users so worth flagging in the notes. |
…108-on-v3.11.x Backport PR #30108 on branch v3.11.x (Fix constrained layout applying pad multiple times)


PR summary
Discovered when I was trying to wrap my head around #30089. Given
On
main, the gap between the rows of the first column is larger when there are more other columnsCurrently the "submerged margins" function works out the required size of the margin by summing the main margin and the "cb" margin (which includes our initial pad). If I have understood, we then apply that required size to only the main margin. So the "cb" margin size gets added on each time we pass through the loop.
Here I just subtract the "cb" values we started with from the target margin size. So the total margin should be what we are aiming for. So the above code now gives:
Given that some test images changed, I'm unsure if this would count as an API change.
PR checklist