-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ENH: pad_inches='layout' for savefig #24981
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
Conversation
lib/matplotlib/transforms.py
Outdated
@@ -616,10 +616,22 @@ def expanded(self, sw, sh): | |||
a = np.array([[-deltaw, -deltah], [deltaw, deltah]]) | |||
return Bbox(self._points + a) | |||
|
|||
def padded(self, p): | |||
"""Construct a `Bbox` by padding this one on all four sides by *p*.""" | |||
def padded(self, pw, ph=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After just expiring a number of deprecations which involved name changes of non-optional arguments, I would suggest adding a @_api.rename_parameter("3.8", "p", "pw")
.
In addition, may one should go all in and spell out the parameter name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I picked very short names for consistency with (some of) the other methods on this class, but maybe w_pad
and h_pad
would be better? That would make them consistent with those for the relatively recently added constrained layout engine.
Thanks for the pointer on _api.rename_parameter
. I tend to forget that the difference between positional and keyword args in python is only pretend!
lib/matplotlib/transforms.py
Outdated
pw : float | ||
Width pad | ||
ph: float, optional | ||
Height pad. Defaults to pw. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Height pad. Defaults to pw. | |
Height pad. Defaults to *pw*. |
I'm not following this feature, but didn't look too closely. Your example calls compressed layout on a single axes (??) and then trims the saved figure by a pad. However, I'm not following why you don't just do fig, axs = plt.subplots(2, 2, layout='compressed')
for ax in axs.flat:
ax.set_aspect(1)
fig.savefig('boo.png', bbox_inches='tight', pad_inches=0.05) I also didn't understand the motivation vis-a-vis a certain width figure? If the user wants a compressed layout and a figure of a certain size they need only do Overall, I'd be very leery of a feature that changes the visual appearance of a figure for just If the motivation is to have different vertical and horizontal padding, why not make that an option to |
My test is not very representative of my target use-case because I was just going for "what's the simplest test I can make that verifies the outer padding looks how I expect". I set the In my real use-case I am more than happy with the layout engine's default pad, so currently I can get what I want with something like fig, axs = plt.subplots(2, 2, figsize=(my_chosen_width, some_large_height), layout='compressed')
for ax in axs.flat:
ax.set_aspect(1)
fig.savefig('boo.png', bbox_inches='tight',
pad_inches=plt.rcParams["figure.constrained_layout.w_pad"]) which was all easy to figure out except what exactly to use for I picked Thinking again this morning, I think my ideal solution would be to change the |
I think modifying |
6e9781e
to
6557bdf
Compare
OK I've now implemented |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is quite right, though I didn't upload to debug yet.
lib/matplotlib/figure.py
Outdated
pad_inches : float, default: :rc:`savefig.pad_inches` | ||
Amount of padding around the figure when bbox_inches is 'tight'. | ||
pad_inches : float or 'layout', default: :rc:`savefig.pad_inches` | ||
Amount of padding around the figure when bbox_inches is 'tight'. If |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amount of padding around the figure when bbox_inches is 'tight'. If | |
Amount of padding in inches around the figure when bbox_inches is 'tight'. If |
h_pad = layout_engine.get()["h_pad"] | ||
w_pad = layout_engine.get()["w_pad"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the layout_engine pads are in figure-normalized units (or at least that is what the docs say, if that isn't the case, then we should fix the docs), whereas bbox_inches='tight' pads are supposed to be in inches...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, the docs do say that (I missed that). But internally the pad gets divided by the figsize
matplotlib/lib/matplotlib/layout_engine.py
Lines 237 to 254 in c23ccdd
def execute(self, fig): | |
""" | |
Perform constrained_layout and move and resize axes accordingly. | |
Parameters | |
---------- | |
fig : `.Figure` to perform layout on. | |
""" | |
width, height = fig.get_size_inches() | |
# pads are relative to the current state of the figure... | |
w_pad = self._params['w_pad'] / width | |
h_pad = self._params['h_pad'] / height | |
return do_constrained_layout(fig, w_pad=w_pad, h_pad=h_pad, | |
wspace=self._params['wspace'], | |
hspace=self._params['hspace'], | |
rect=self._params['rect'], | |
compress=self._compress) |
so I think the docs may indeed be wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oooops, thats not good. I guess padding in inches (or points) makes more sense to me (ahem, and probably did when I wrote this) and the docs need to be fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The matplotlibrc is OK though
#figure.constrained_layout.h_pad: 0.04167 # Padding around axes objects. Float representing
#figure.constrained_layout.w_pad: 0.04167 # inches. Default is 3/72 inches (3 points)
ded0868
to
a6b6dd9
Compare
Maybe a bit too early to approve as I think there should go some |
Should this get a what's new entry? (ooops, cross post) |
Looks like the criteria for a I'm not sure I would class this as "major"? If it should get a what's new entry, do we need to wait till after the 3.7 release - since this is milestoned for 3.8 and the whatsnew directory is currently full of 3.7 stuff? |
No, the branch is made; anything for 3.8 is fine to put in |
I wouldn't add a version added note, because it's just a new argument to the keyword. But I'd add to the what's-new. |
8c51c42
to
59cc1b2
Compare
59cc1b2
to
1eb1323
Compare
PR Summary
When using constrained or compressed layout,
will use the padding defined on the layout engine.
Since the layout engine has configurable padding information on it, it makes sense to me that there should be a simple way to tell
savefig
to use that. This option means that either the height or width of the output file will match what was set atfigsize
. When preparing figures for a paper, a user might be particularly fussy about the width. Note that the default pad forbbox_inches='tight'
is larger than the default for the layout engine, so currently a little fiddling is required to achieve that (I told myself that I shouldn't really care about 3mm but...).PR Checklist
Documentation and Tests
pytest
passes)Release Notes
.. versionadded::
directive in the docstring and documented indoc/users/next_whats_new/
.. versionchanged::
directive in the docstring and documented indoc/api/next_api_changes/
next_whats_new/README.rst
ornext_api_changes/README.rst