Convert **kwargs to named arguments for a clearer API by Zac-HD · Pull Request #11137 · matplotlib/matplotlib · GitHub
Skip to content

Convert **kwargs to named arguments for a clearer API #11137

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

Merged
timhoffm merged 4 commits into from
May 8, 2018
Merged

Convert **kwargs to named arguments for a clearer API #11137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6be9c9f
Use named args for xticks, yticks in pyplot
Zac-HD Apr 27, 2018
09991e6
Clarify and check alternative names for axis limits
Zac-HD Apr 27, 2018
5720387
Minor refactor for clarity
Zac-HD Apr 27, 2018
c408392
Eliminate unused variables in axes3d
Zac-HD Apr 27, 2018
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
14 changes: 14 additions & 0 deletions doc/api/next_api_changes/2018-04-22-ZHD.rst
View file
Open in desktop
70 changes: 44 additions & 26 deletions lib/matplotlib/axes/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2756,8 +2756,6 @@ def ticklabel_format(self, *, axis='both', style='', scilimits=None,
sb = True
elif style == 'plain':
sb = False
elif style == 'comma':
raise NotImplementedError("comma style remains to be added")

@tacaswell tacaswell Apr 28, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add an API changes note for this? I don't think we need to anything about it code-wise, but should document that an exception type has changed.

@Zac-HD Zac-HD Apr 29, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

elif style == '':
sb = None
else:
Expand Down Expand Up @@ -3028,7 +3026,8 @@ def _validate_converted_limits(self, limit, convert):
raise ValueError("Axis limits cannot be NaN or Inf")
return converted_limit

def set_xlim(self, left=None, right=None, emit=True, auto=False, **kw):
def set_xlim(self, left=None, right=None, emit=True, auto=False,
*, xmin=None, xmax=None):

@dstansby dstansby Apr 28, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it be worth deprecating xmin and xmax or left or right? Seems a bit silly having two ways of specifying what is really the same argument.

@timhoffm timhoffm Apr 28, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 that two ways are a bit silly. Not sure if we have to go as far as deprecating (but wouldn‘t object either. Maybe go slow with a PendingDeprecation). Anyway, there should be one recommended way and the other only is only a backward compatible fallback.

The question is which to recommend:

  • xmin, xmax is more generic and easy to transfer, e.g. ymin rmin
  • left, right is more semantic

To me +0.5 for keeping xmin, xmax

@anntzer anntzer Apr 28, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually, left/right is clearly better IMO: one can pass left > right for inverted axes, which sounds less silly that xmin > xmax...

@tacaswell tacaswell Apr 28, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This code path is due to an API change from @efiring in 2010 (9ca5db0) and this has not been documented in the docstring. I am inclined to deprecate these unless we find that they are widely used in the wild.

I am also 👍 on pushing the deprecation work to a different PR.

"""
Set the data limits for the x-axis

Expand All @@ -3039,6 +3038,9 @@ def set_xlim(self, left=None, right=None, emit=True, auto=False, **kw):
left : scalar, optional
The left xlim (default: None, which leaves the left limit
unchanged).
The left and right xlims may be passed as the tuple
(`left`, `right`) as the first positional argument (or as
the `left` keyword argument).

right : scalar, optional
The right xlim (default: None, which leaves the right limit
Expand All @@ -3051,10 +3053,11 @@ def set_xlim(self, left=None, right=None, emit=True, auto=False, **kw):
Whether to turn on autoscaling of the x-axis. True turns on,
False turns off (default action), None leaves unchanged.

xlimits : tuple, optional
The left and right xlims may be passed as the tuple
(`left`, `right`) as the first positional argument (or as
the `left` keyword argument).
xmin, xmax : scalar, optional
These arguments are deprecated and will be removed in a future
version. They are equivalent to left and right respectively,
and it is an error to pass both `xmin` and `left` or
`xmax` and `right`.

Returns
-------
Expand Down Expand Up @@ -3085,15 +3088,20 @@ def set_xlim(self, left=None, right=None, emit=True, auto=False, **kw):
>>> set_xlim(5000, 0)

"""
if 'xmin' in kw:
left = kw.pop('xmin')
if 'xmax' in kw:
right = kw.pop('xmax')
if kw:
raise ValueError("unrecognized kwargs: %s" % list(kw))

if right is None and iterable(left):
left, right = left
if xmin is not None:
cbook.warn_deprecated('3.0', name='`xmin`',
alternative='`left`', obj_type='argument')
if left is not None:
raise TypeError('Cannot pass both `xmin` and `left`')
left = xmin
if xmax is not None:
cbook.warn_deprecated('3.0', name='`xmax`',
alternative='`right`', obj_type='argument')
if right is not None:
raise TypeError('Cannot pass both `xmax` and `right`')
right = xmax

self._process_unit_info(xdata=(left, right))
left = self._validate_converted_limits(left, self.convert_xunits)
Expand Down Expand Up @@ -3358,7 +3366,8 @@ def get_ylim(self):
"""
return tuple(self.viewLim.intervaly)

def set_ylim(self, bottom=None, top=None, emit=True, auto=False, **kw):
def set_ylim(self, bottom=None, top=None, emit=True, auto=False,
*, ymin=None, ymax=None):
"""
Set the data limits for the y-axis

Expand All @@ -3369,6 +3378,9 @@ def set_ylim(self, bottom=None, top=None, emit=True, auto=False, **kw):
bottom : scalar, optional
The bottom ylim (default: None, which leaves the bottom
limit unchanged).
The bottom and top ylims may be passed as the tuple
(`bottom`, `top`) as the first positional argument (or as
the `bottom` keyword argument).

top : scalar, optional
The top ylim (default: None, which leaves the top limit
Expand All @@ -3381,10 +3393,11 @@ def set_ylim(self, bottom=None, top=None, emit=True, auto=False, **kw):
Whether to turn on autoscaling of the y-axis. True turns on,
False turns off (default action), None leaves unchanged.

ylimits : tuple, optional
The bottom and top yxlims may be passed as the tuple
(`bottom`, `top`) as the first positional argument (or as
the `bottom` keyword argument).
ymin, ymax : scalar, optional
These arguments are deprecated and will be removed in a future
version. They are equivalent to bottom and top respectively,
and it is an error to pass both `xmin` and `bottom` or
`xmax` and `top`.

Returns
-------
Expand Down Expand Up @@ -3414,15 +3427,20 @@ def set_ylim(self, bottom=None, top=None, emit=True, auto=False, **kw):

>>> set_ylim(5000, 0)
"""
if 'ymin' in kw:
bottom = kw.pop('ymin')
if 'ymax' in kw:
top = kw.pop('ymax')
if kw:
raise ValueError("unrecognized kwargs: %s" % list(kw))

if top is None and iterable(bottom):
bottom, top = bottom
if ymin is not None:
cbook.warn_deprecated('3.0', name='`ymin`',
alternative='`bottom`', obj_type='argument')
if bottom is not None:
raise TypeError('Cannot pass both `ymin` and `bottom`')
bottom = ymin
if ymax is not None:
cbook.warn_deprecated('3.0', name='`ymax`',
alternative='`top`', obj_type='argument')
if top is not None:
raise TypeError('Cannot pass both `ymax` and `top`')
top = ymax

bottom = self._validate_converted_limits(bottom, self.convert_yunits)
top = self._validate_converted_limits(top, self.convert_yunits)
Expand Down
10 changes: 4 additions & 6 deletions lib/matplotlib/axis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,12 +334,10 @@ def get_view_interval(self):
raise NotImplementedError('Derived must override')

def _apply_params(self, **kw):
switchkw = ['gridOn', 'tick1On', 'tick2On', 'label1On', 'label2On']
switches = [k for k in kw if k in switchkw]
for k in switches:
setattr(self, k, kw.pop(k))
newmarker = [k for k in kw if k in ['size', 'width', 'pad', 'tickdir']]
if newmarker:
for name in ['gridOn', 'tick1On', 'tick2On', 'label1On', 'label2On']:
if name in kw:
setattr(self, name, kw.pop(name))
if any(k in kw for k in ['size', 'width', 'pad', 'tickdir']):
self._size = kw.pop('size', self._size)
# Width could be handled outside this block, but it is
# convenient to leave it here.
Expand Down
36 changes: 16 additions & 20 deletions lib/matplotlib/pyplot.py
View file
Open in desktop
Loading