Skip to content

Fix constrained layout applying pad multiple times#30108

Merged
tacaswell merged 1 commit into
matplotlib:mainfrom
rcomer:submerged-margins
May 11, 2026
Merged

Fix constrained layout applying pad multiple times#30108
tacaswell merged 1 commit into
matplotlib:mainfrom
rcomer:submerged-margins

Conversation

@rcomer
Copy link
Copy Markdown
Member

@rcomer rcomer commented May 25, 2025

PR summary

Discovered when I was trying to wrap my head around #30089. Given

import matplotlib.pyplot as plt

mosaic1 = "AC;BC"
mosaic2 = "ACDE;BCDE"

for mosaic in mosaic1, mosaic2:
    fig, ax_dir = plt.subplot_mosaic(mosaic, layout='constrained', facecolor='skyblue')
    fig.get_layout_engine().set(h_pad=0.2)
    
plt.show()

On main, the gap between the rows of the first column is larger when there are more other columns

image
image

Currently 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:

image
image

Given that some test images changed, I'm unsure if this would count as an API change.

PR checklist

@github-actions github-actions Bot added the topic: geometry manager LayoutEngine, Constrained layout, Tight layout label May 25, 2025
@jklymak
Copy link
Copy Markdown
Member

jklymak commented May 25, 2025

This seems OK. However, can we make sure it works when there actually is a colorbar? (cb is shorthand for the colorbar margins)

@rcomer
Copy link
Copy Markdown
Member Author

rcomer commented May 25, 2025

Something like this?

import matplotlib.pyplot as plt

mosaic = "AABBCC;DDDEEE"

fig, ax_dir = plt.subplot_mosaic(mosaic, layout='constrained', facecolor='skyblue')

cf = ax_dir['A'].contourf([[0, 1], [2, 3]])
fig.colorbar(cf)
    
plt.show()

image

@jklymak
Copy link
Copy Markdown
Member

jklymak commented May 26, 2025

Hmm, if I dont' have the submerged axes then I get
Figure_1

But I guess we want the middle axes centred on the bottom two and the same size as the two to the right, so maybe this PR good?

@QuLogic QuLogic changed the title Fix contrained layout applying pad multiple times Fix constrained layout applying pad multiple times Apr 17, 2026
@rcomer rcomer linked an issue May 4, 2026 that may be closed by this pull request
@rcomer rcomer force-pushed the submerged-margins branch from 06c96f1 to cbfbfbf Compare May 4, 2026 19:02
@rcomer
Copy link
Copy Markdown
Member Author

rcomer commented May 4, 2026

Rebased and got linting failures I don't understand. They are about rst files.

@rcomer
Copy link
Copy Markdown
Member Author

rcomer commented May 4, 2026

Does this need a test for a case with a colorbar, and if so is an image comparison test based on #30108 (comment) enough?

@rcomer rcomer force-pushed the submerged-margins branch from cbfbfbf to 861afdf Compare May 4, 2026 19:13
@jklymak
Copy link
Copy Markdown
Member

jklymak commented May 4, 2026

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

@rcomer rcomer marked this pull request as draft May 4, 2026 21:15
@rcomer rcomer force-pushed the submerged-margins branch from 861afdf to 10f1435 Compare May 8, 2026 18:57
@rcomer rcomer marked this pull request as ready for review May 8, 2026 19:50
@rcomer rcomer force-pushed the submerged-margins branch from 10f1435 to 1486387 Compare May 8, 2026 20:06
Copy link
Copy Markdown
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

The change seems reasonable and the updated figures don't look broken.

@timhoffm timhoffm added this to the v3.11.0 milestone May 9, 2026
@timhoffm
Copy link
Copy Markdown
Member

timhoffm commented May 9, 2026

@QuLogic can tihs still go into 3.11?

Because it's a visual change, do we want to mention it in the release notes?

@QuLogic
Copy link
Copy Markdown
Member

QuLogic commented May 9, 2026

I think we may still have some room for it.

@rcomer
Copy link
Copy Markdown
Member Author

rcomer commented May 9, 2026

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?

@rcomer rcomer force-pushed the submerged-margins branch from 1486387 to 760068c Compare May 9, 2026 08:06
@rcomer
Copy link
Copy Markdown
Member Author

rcomer commented May 9, 2026

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.

@rcomer rcomer force-pushed the submerged-margins branch from 760068c to 8ac4071 Compare May 10, 2026 17:44
@tacaswell tacaswell merged commit 93e7277 into matplotlib:main May 11, 2026
40 of 41 checks passed
QuLogic added a commit that referenced this pull request May 11, 2026
…108-on-v3.11.x

Backport PR #30108 on branch v3.11.x (Fix constrained layout applying pad multiple times)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: geometry manager LayoutEngine, Constrained layout, Tight layout

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Constrained Layout scaling of layouts with submerged spines

5 participants