Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Fix padding and spacing for GridSpecFromSubplotSpec with layout="constrained" #28895

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
Loading
from
Open

Fix padding and spacing for GridSpecFromSubplotSpec with layout="constrained" #28895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9e7a6bf
Ensure that `GridSpecFromSubplotSpec` instances are contained within …
jakelevi1996 Sep 26, 2024
ccd482f
Ensure that padding around the edges of `GridSpecFromSubplotSpec` is …
jakelevi1996 Sep 26, 2024
48239ff
Add unit test `test_gridspecfromsubplotspec_wspace_hspace`
jakelevi1996 Sep 26, 2024
852d439
Add unit test `test_gridspecfromsubplotspec_pad`
jakelevi1996 Sep 26, 2024
559dddb
Restore previous appearance by using nested gridspecs
jakelevi1996 Sep 26, 2024
d20373e
Turn off axis ticks
jakelevi1996 Sep 26, 2024
694b919
Use deterministic image, and specify colour map
jakelevi1996 Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions 13 lib/matplotlib/_constrained_layout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
from matplotlib import _api, artist as martist
import matplotlib.transforms as mtransforms
import matplotlib._layoutgrid as mlayoutgrid
import matplotlib.gridspec as mgridspec


_log = logging.getLogger(__name__)
Expand Down Expand Up @@ -232,6 +233,7 @@ def make_layoutgrids_gs(layoutgrids, gs):
if rep not in layoutgrids:
layoutgrids[rep] = mlayoutgrid.LayoutGrid(
parent=subspeclb,
parent_inner=True,
name='top',
nrows=1, ncols=1,
parent_pos=(subplot_spec.rowspan, subplot_spec.colspan))
Expand Down Expand Up @@ -325,6 +327,17 @@ def get_margin_from_padding(obj, *, w_pad=0, h_pad=0,
'bottomcb': h_pad, 'topcb': h_pad,
'left': 0, 'right': 0,
'top': 0, 'bottom': 0}

if isinstance(gs, mgridspec.GridSpecFromSubplotSpec):
Copy link
Member

Choose a reason for hiding this comment

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

As noted in the issue, I'm not in favour of this change. margins are to be relative to their outer container. If the outer container also has a margin that should be respected as well.

I understand aesthetically what you are trying to do. However, you are basically overriding the point of a margin, which other users may have set this way on purpose. Secondly, in a practical example with labels these axes will almost certainly not be lined up the way you are suggesting anyways.

Copy link
Author

Choose a reason for hiding this comment

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

I've come up with an alternative solution to (hopefully) address these issues and opened a new PR: #28903

See images in that PR which demonstrate testing with randomly sized labels, both before and after the new fix is applied.

if ss.is_first_col():
margin['leftcb'] = 0
if ss.is_last_col():
margin['rightcb'] = 0
if ss.is_first_row():
margin['topcb'] = 0
if ss.is_last_row():
margin['bottomcb'] = 0

if _wspace / ncols > w_pad:
if ss.colspan.start > 0:
margin['leftcb'] = _wspace / ncols
Expand Down
Binary file added BIN +5.19 KB ...ib/tests/baseline_images/test_constrainedlayout/gridspecfromsubplotspec_pad.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added BIN +5.14 KB ...tests/baseline_images/test_constrainedlayout/gridspecfromsubplotspec_wspace.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
70 changes: 66 additions & 4 deletions 70 lib/matplotlib/tests/test_constrainedlayout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,39 +211,45 @@ def test_constrained_layout10():
def test_constrained_layout11():
"""Test for multiple nested gridspecs"""

def inner_sp(sp):
return gridspec.GridSpecFromSubplotSpec(1, 1, sp)[0]

fig = plt.figure(layout="constrained", figsize=(13, 3))
gs0 = gridspec.GridSpec(1, 2, figure=fig)
gsl = gridspec.GridSpecFromSubplotSpec(1, 2, gs0[0])
gsl0 = gridspec.GridSpecFromSubplotSpec(2, 2, gsl[1])
ax = fig.add_subplot(gs0[1])
ax = fig.add_subplot(inner_sp(gs0[1]))
example_plot(ax, fontsize=9)
axs = []
for gs in gsl0:
ax = fig.add_subplot(gs)
axs += [ax]
pcm = example_pcolor(ax, fontsize=9)
fig.colorbar(pcm, ax=axs, shrink=0.6, aspect=70.)
ax = fig.add_subplot(gsl[0])
ax = fig.add_subplot(inner_sp(gsl[0]))
Copy link
Member

Choose a reason for hiding this comment

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

This is a huge breaking change. We can't force users to do this!

example_plot(ax, fontsize=9)


@image_comparison(['constrained_layout11rat.png'])
def test_constrained_layout11rat():
"""Test for multiple nested gridspecs with width_ratios"""

def inner_sp(sp):
return gridspec.GridSpecFromSubplotSpec(1, 1, sp)[0]

fig = plt.figure(layout="constrained", figsize=(10, 3))
gs0 = gridspec.GridSpec(1, 2, figure=fig, width_ratios=[6, 1])
gsl = gridspec.GridSpecFromSubplotSpec(1, 2, gs0[0])
gsl0 = gridspec.GridSpecFromSubplotSpec(2, 2, gsl[1], height_ratios=[2, 1])
ax = fig.add_subplot(gs0[1])
ax = fig.add_subplot(inner_sp(gs0[1]))
example_plot(ax, fontsize=9)
axs = []
for gs in gsl0:
ax = fig.add_subplot(gs)
axs += [ax]
pcm = example_pcolor(ax, fontsize=9)
fig.colorbar(pcm, ax=axs, shrink=0.6, aspect=70.)
ax = fig.add_subplot(gsl[0])
ax = fig.add_subplot(inner_sp(gsl[0]))
example_plot(ax, fontsize=9)


Expand Down Expand Up @@ -719,3 +725,59 @@ def test_layout_leak():
gc.collect()
assert not any(isinstance(obj, mpl._layoutgrid.LayoutGrid)
for obj in gc.get_objects())


@image_comparison(['gridspecfromsubplotspec_wspace.png'])
def test_gridspecfromsubplotspec_wspace_hspace():
fig = plt.figure(figsize=[10, 6], layout="constrained")
a0, a1 = fig.subplots(
nrows=1,
ncols=2,
gridspec_kw={"wspace": 0.3, "hspace": 0.3},
width_ratios=[2, 1],
)

for axis in [a0, a1]:
axis.get_xaxis().set_visible(False)
axis.get_yaxis().set_visible(False)
Comment on lines +741 to +742
Copy link
Member

Choose a reason for hiding this comment

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

The goal of constrained_layout is to deal with varying sized labels on axes. You should not remove the tick labels. Ideally you would also test with randomly sized labels, so in this case the inner labels could be shorter than the outer labels.

for s in axis.spines.values():
s.set(color="r", lw=10)

subplot_spec = axis.get_subplotspec()
subgrid_spec = subplot_spec.subgridspec(
nrows=3,
ncols=3,
wspace=0.1,
hspace=0.1,
)
axis_array = subgrid_spec.subplots()
for a in axis_array.flat:
a.get_xaxis().set_visible(False)
a.get_yaxis().set_visible(False)


@image_comparison(['gridspecfromsubplotspec_pad.png'])
def test_gridspecfromsubplotspec_pad():
def add_subplots(axis, nx, ny):
axis.clear()
axis.set_axis_off()
subplot_spec = axis.get_subplotspec()
subgrid_spec = subplot_spec.subgridspec(nx, ny)
axis_list = subgrid_spec.subplots().flatten().tolist()
for a in axis_list:
show_random_image(a)

return axis_list

def show_random_image(axis):
z = np.linspace(0, 1, 30).reshape(10, 3)
axis.pcolormesh(z, cmap="plasma")
axis.set_axis_off()

fig = plt.figure(figsize=[10, 6], layout="constrained")
fig.get_layout_engine().set(w_pad=0.2, h_pad=0.2)

axes = fig.subplots(1, 2, width_ratios=[1, 2])
show_random_image(axes[0])
axes = add_subplots(axes[1], 2, 2)
axes = add_subplots(axes[1], 2, 2)
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.