Skip to content

[Bug]: wspace and hspace in subfigures not working #25511

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.

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

Closed
maurosilber opened this issue Mar 20, 2023 · 17 comments · Fixed by #25960
Closed

[Bug]: wspace and hspace in subfigures not working #25511

maurosilber opened this issue Mar 20, 2023 · 17 comments · Fixed by #25960

Comments

@maurosilber
Copy link

Bug summary

wspace and hspace in Figure.subfigures do nothing.

Code for reproduction

import matplotlib.pyplot as plt

figs = plt.figure().subfigures(2, 2, wspace=0, hspace=0)
for fig in figs.flat:
    fig.subplots().plot([1, 2])
plt.show()

Actual outcome

Same figure independently of the values of hspace and wspace.

Expected outcome

wspace, hspace : float, default: None
The amount of width/height reserved for space between subfigures,
expressed as a fraction of the average subfigure width/height.
If not given, the values will be inferred from a figure or
rcParams when necessary.

Additional information

No response

Operating system

OS/X

Matplotlib Version

3.7.1

Matplotlib Backend

MacOSX

Python version

Python 3.10.9

Jupyter version

No response

Installation

conda

@rcomer
Copy link
Member

rcomer commented Mar 20, 2023

Thanks for the report @maurosilber. The problem is clearer if we set a facecolor for each subfigure:

import matplotlib.pyplot as plt

for space in [0, 0.2]:
    figs = plt.figure().subfigures(2, 2, hspace=space, wspace=space)
    for fig, color in zip(figs.flat, 'cmyw'):
        fig.set_facecolor(color)
        fig.subplots().plot([1, 2])
plt.show()

With main, both figures look like
test

@Miguel-LlamasLanza
Copy link

Just to add something, it looks like it works when using 'constrained' layout.

@rcomer rcomer removed this from the v3.7.2 milestone Mar 20, 2023
@jklymak
Copy link
Member

jklymak commented Mar 21, 2023

The relevant code is

def _redo_transform_rel_fig(self, bbox=None):

This didn't break - it never worked. I imagine the position logic could be borrowed from Axes....

@dswelch
Copy link

dswelch commented Mar 25, 2023

I am a first time contributer, and would like to attempt to work on this if possible! Will get a PR in the coming days

@dswelch
Copy link

dswelch commented Mar 29, 2023

I have been trying to understand the code associated with this and have run into some issues.
In the function _redo_transform_rel_fig (linked above), I feel that I would have to be able to access all of the subfigures within this figure in order to give the correct amount of space based on the average subfigure width/height. Would this be possible?
I have been looking to this function as inspiration for the logic, but I am still trying to understand all the parameters as well:

def get_grid_positions(self, fig, raw=False):

@tacaswell
Copy link
Member

There is a fig.subfigs attribute which is a list of the SubFigures in a Figure.

@dswelch
Copy link

dswelch commented Apr 4, 2023

Apologies for the slow progress, had a busier week than expected.
Below is the code I am running to test.

import matplotlib.pyplot as plt

figs = plt.figure().subfigures(2, 2, hspace=0.2, wspace=0.2)
for fig, color in zip(figs.flat, 'cmyw'):
    fig.set_facecolor(color)
    fig.subplots().plot([1, 2])
# plt.show()

figs = plt.figure(constrained_layout=True).subfigures(2, 2, hspace=0.2, wspace=0.2)
for fig, color in zip(figs.flat, 'cmyw'):
    fig.set_facecolor(color)
    fig.subplots().plot([1, 2])
plt.show()

This creates two figures, one with constrained layout and one without. Below is my current output.
On the right is the constrained layout figure, and the left is the one without.
Screenshot 2023-04-04 at 6 20 33 PM

My code currently fits the facecolors in the background to the correct spots, however the actual graphs do not match. They seem to need resizing to the right and upwards in order to match the constrained layout. Would a non-constrained layout figure be expected to resize those graphs to fit the background? I would assume so but I wanted to check since I couldn't find the answer in the documentation I looked at.

@jklymak
Copy link
Member

jklymak commented Apr 4, 2023

My code currently fits the facecolors in the background to the correct spots, however the actual graphs do not match. They seem to need resizing to the right and upwards in order to match the constrained layout. Would a non-constrained layout figure be expected to resize those graphs to fit the background? I would assume so but I wanted to check since I couldn't find the answer in the documentation I looked at.

I'm not quite sure what you are asking here? Constrained layout adjusts the axes sizes to fit in the figure. If you don't do constrained layout the axes labels can definitely spill out of the figure if you just use default axes positioning.

@rcomer
Copy link
Member

rcomer commented May 20, 2023

I've been digging into this. We have a test that adds a subplot and a subfigure using the same gridspec, and the subfigure is expected to ignore the wspace on the gridspec.

@image_comparison(['test_subfigure_scatter_size.png'], style='mpl20',
remove_text=True)
def test_subfigure_scatter_size():
# markers in the left- and right-most subplots should be the same
fig = plt.figure()
gs = fig.add_gridspec(1, 2)
ax0 = fig.add_subplot(gs[1])
ax0.scatter([1, 2, 3], [1, 2, 3], s=30, marker='s')
ax0.scatter([3, 4, 5], [1, 2, 3], s=[20, 30, 40], marker='s')
sfig = fig.add_subfigure(gs[0])
axs = sfig.subplots(1, 2)
for ax in [ax0, axs[0]]:
ax.scatter([1, 2, 3], [1, 2, 3], s=30, marker='s', color='r')
ax.scatter([3, 4, 5], [1, 2, 3], s=[20, 30, 40], marker='s', color='g')

The use-case envisioned in the test seems entirely reasonable to me, but I'm struggling to see how we can support that while also fixing this issue.

@jklymak
Copy link
Member

jklymak commented May 22, 2023

Why do you say the subfigure is expected to ignore the wspace? I don't see that wspace is set in the test.

@rcomer
Copy link
Member

rcomer commented May 22, 2023

Since no wspace is passed, I assume the gridspec will have the default from rcParams, which is 0.2.

@jklymak
Copy link
Member

jklymak commented May 23, 2023

Sure, but I don't understand what wouldn't work in that example with wspace argument. Do you just mean that the default would be too large for this case?

@rcomer
Copy link
Member

rcomer commented May 23, 2023

Yes, I think in the test example, if both subfigure and subplot were respecting the 0.2 wspace then the left-hand subplots would be narrower and we’d have more whitespace in the middle. Currently in this example the total width of the two lefthand subplots looks about the same as the width of the righthand one, so overall the figure seems well-balanced.

Another test here explicitly doesn’t expect any whitespace between subfigures, though in this case there are no subplots so you could just pass wspace=0, hspace=0 to the gridspec and retain this result.

def test_subfigure_spanning():
# test that subfigures get laid out properly...
fig = plt.figure(constrained_layout=True)
gs = fig.add_gridspec(3, 3)
sub_figs = [
fig.add_subfigure(gs[0, 0]),
fig.add_subfigure(gs[0:2, 1]),
fig.add_subfigure(gs[2, 1:3]),
fig.add_subfigure(gs[0:, 1:])
]
w = 640
h = 480
np.testing.assert_allclose(sub_figs[0].bbox.min, [0., h * 2/3])
np.testing.assert_allclose(sub_figs[0].bbox.max, [w / 3, h])
np.testing.assert_allclose(sub_figs[1].bbox.min, [w / 3, h / 3])
np.testing.assert_allclose(sub_figs[1].bbox.max, [w * 2/3, h])
np.testing.assert_allclose(sub_figs[2].bbox.min, [w / 3, 0])
np.testing.assert_allclose(sub_figs[2].bbox.max, [w, h / 3])

@jklymak
Copy link
Member

jklymak commented May 23, 2023

Can we just make the default wspace for subfigures be zero?

@rcomer
Copy link
Member

rcomer commented May 23, 2023

wspace is a property of the gridspec. Do you mean we should have a separate property for subfigures, e.g. GridSpec.subfig_wspace, with its own default?

@jklymak
Copy link
Member

jklymak commented May 23, 2023

gridspec is still public API, but barely, as there are usually more elegant ways to do the same things that folks used to use gridspecs for.

In this case, as you point out, it is better if subplots and subfigures get different wspace values, even if they are the same grid spec level. I'm suggesting that subfigures ignores the grid spec wspace (as it currently does) and if we want a wspace for a set of subfigures that be a kwarg of the subfigure call.

However, I never use wspace nor hspace, and given that all these things work so much better with constrained layout, I'm not sure what the goal of manually tweaking the spacing is.

@rcomer
Copy link
Member

rcomer commented May 23, 2023

OK, how’s this? #25960

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants