Skip to content

[Bug]: constrained_layout_pads as kwargs of Figure #22029

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

Open
StefRe opened this issue Dec 21, 2021 · 5 comments
Open

[Bug]: constrained_layout_pads as kwargs of Figure #22029

StefRe opened this issue Dec 21, 2021 · 5 comments

Comments

@StefRe
Copy link
Contributor

StefRe commented Dec 21, 2021

Bug summary

According to the figure documentation (other parameters) it should be possible pass figure kwargs in plt.subplots():

fig, ax = plt.subplots(layout='constrained',
                       constrained_layout_pads={'w_pad': 0.5, 'h_pad': 0.5})

which results, however, in a

TypeError: set_constrained_layout_pads() takes 1 positional argument but 2 were given

The documentation mentions a single value (float), although the parameter name is in plural (and set_constrained_layout_pads takes up to four parameters):

constrained_layout_pads float, default: rcParams["figure.constrained_layout.w_pad"] (default: 0.04167)

but plt.subplots(constrained_layout_pads=0.5) results in the same error.

Code for reproduction

import matplotlib.pyplot as plt

fig, ax = plt.subplots(layout='constrained',
                       constrained_layout_pads={'w_pad': 0.5, 'h_pad': 0.5})

Actual outcome


  File "/usr/lib/python3.10/site-packages/matplotlib/pyplot.py", line 1434, in subplots
    fig = figure(**fig_kw)

  File "/usr/lib/python3.10/site-packages/matplotlib/pyplot.py", line 787, in figure
    manager = new_figure_manager(

  File "/usr/lib/python3.10/site-packages/matplotlib/pyplot.py", line 306, in new_figure_manager
    return _backend_mod.new_figure_manager(*args, **kwargs)

  File "/usr/lib/python3.10/site-packages/matplotlib/backend_bases.py", line 3493, in new_figure_manager
    fig = fig_cls(*args, **kwargs)

  File "/usr/lib/python3.10/site-packages/matplotlib/figure.py", line 2238, in __init__
    super().__init__(**kwargs)

  File "/usr/lib/python3.10/site-packages/matplotlib/figure.py", line 216, in __init__
    self.set(**kwargs)

  File "/usr/lib/python3.10/site-packages/matplotlib/artist.py", line 116, in <lambda>
    cls.set = lambda self, **kwargs: Artist.set(self, **kwargs)

  File "/usr/lib/python3.10/site-packages/matplotlib/artist.py", line 1164, in set
    return self.update(kwargs)

  File "/usr/lib/python3.10/site-packages/matplotlib/artist.py", line 1066, in update
    ret.append(func(v))

TypeError: Figure.set_constrained_layout_pads() takes 1 positional argument but 2 were given

Expected outcome

no error

Additional information

also discussed at discours

Operating system

Arch

Matplotlib Version

3.5.1

Matplotlib Backend

Qt5Agg

Python version

3.10.1

Jupyter version

No response

Installation

Linux package manager

@jklymak
Copy link
Member

jklymak commented Dec 21, 2021

Thanks for opening this here. See also #20115 (comment)

@tacaswell
Copy link
Member

xref #20115 (comment)

Matplotlib Artist instances have many properties (approximately the set given by the getter/setters (and we have looked into moving to properties / traitlets / ... for more managed access to them and there are some edge cases like this one that make it hard). As a nice short hand we have a convention that usually **kwargs to the __init__ method of the Artist sub-classes get tossed into self.update which is

def update(self, props):
"""
Update this artist's properties from the dict *props*.
Parameters
----------
props : dict
"""
ret = []
with cbook._setattr_cm(self, eventson=False):
for k, v in props.items():
# Allow attributes we want to be able to update through
# art.update, art.set, setp.
if k == "axes":
ret.append(setattr(self, k, v))
else:
func = getattr(self, f"set_{k}", None)
if not callable(func):
raise AttributeError(f"{type(self).__name__!r} object "
f"has no property {k!r}")
ret.append(func(v))
if ret:
self.pchanged()
self.stale = True
return ret

Thus we can write things like

ax.plot(x, y, linewidth=5, color='green')

rather than

ln, = ax.plot(x, y)
ln.set_linewidth(5)
ln.set_color('green')

This works on the expectation that a) for a keyword foo there is a method set_foo on the instance b) the method will take 1 positional argument (the value of the keyword) that can be passed in and "do the right thing" (that some of the set_foo methods violate this and take optional arguments is actually one of the biggest sticking points in moving to property-like usage).

One fix would be to exclude constrained_layout_pads from functions that are allowed to be passed to via kwargs (so you would get a clearer exception), but given that we are going to be py38+ for our next release, we have access to positional-only arguements so we can change the signature to

    def set_constrained_layout_pads(self, d=None, /, *,
                                    w_pad=None, h_pad=None,
                                    wspace=None, hspace=None):

which along with a bit of munging in body can make this work as @StefRe expected.

@jklymak
Copy link
Member

jklymak commented Dec 21, 2021

Right, the unexpected thing for me was that this turned up in the docs - obviously we never meant this to be a kwarg for subplots, even if someone could have called it if they knew what to look for. I guess we could have easily made set_constrained_layout_pads something that didn't start with set.

Anyhow, I'm fine w/ the going forward 3.8+ plan. It'd be great to accept a dict as a kwarg. I'm a little concerned that we need to do something for MPL 3.5.x about the documentation. Can we just define FigureBase.set locally, filter out constrained_layout_pads for now, and remember to revert it for 3.6?

@tacaswell
Copy link
Member

I think if we go with @anntzer 's suggestion of filtering on bind(None) rather than parameter count, then we can keep that in place going forward and add the positional-only to make this work for mpl36.

@tacaswell tacaswell added this to the v3.5.2 milestone Dec 22, 2021
@timhoffm
Copy link
Member

timhoffm commented Apr 5, 2022

This will be easier to handle once we define artist properties explicitly. Xref proposal in #22749

@QuLogic QuLogic modified the milestones: v3.5.2, v3.5.3 Apr 30, 2022
@QuLogic QuLogic modified the milestones: v3.5.3, v3.5.4 Jul 21, 2022
@oscargus oscargus modified the milestones: v3.5.4, v3.6.1 Sep 25, 2022
@QuLogic QuLogic modified the milestones: v3.6.1, v3.6.2 Oct 6, 2022
@QuLogic QuLogic modified the milestones: v3.6.2, v3.6.3 Oct 27, 2022
@QuLogic QuLogic modified the milestones: v3.6.3, future releases Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants