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.

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
merged 4 commits into from
May 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
Different exception types for undocumented options
--------------------------------------------------

- Passing ``style='comma'`` to :meth:`~matplotlib.axes.Axes.ticklabel_format`
was never supported. It now raises ``ValueError`` like all other
unsupported styles, rather than ``NotImplementedError``.

- Passing the undocumented ``xmin`` or ``xmax`` arguments to
:meth:`~matplotlib.axes.Axes.set_xlim` would silently override the ``left``
and ``right`` arguments. :meth:`~matplotlib.axes.Axes.set_ylim` and the
3D equivalents (e.g. :meth:`~mpl_toolkits.axes.Axes3D.set_zlim3d`) had a
corresponding problem.
The ``_min`` and ``_max`` arguments are now deprecated, and a ``TypeError``
will be raised if they would override the earlier limit arguments.
70 changes: 44 additions & 26 deletions lib/matplotlib/axes/_base.py
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")
Copy link
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.

Copy link
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):
Copy link
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.

Copy link
Member

@timhoffm timhoffm Apr 28, 2018

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

Copy link
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...

Copy link
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -1365,19 +1365,19 @@ def ylim(*args, **kwargs):
return ret


def xticks(*args, **kwargs):
def xticks(ticks=None, labels=None, **kwargs):
"""
Get or set the current tick locations and labels of the x-axis.

Call signatures::

locs, labels = xticks() # Get locations and labels

xticks(locs, [labels], **kwargs) # Set locations and labels
xticks(ticks, [labels], **kwargs) # Set locations and labels

Parameters
----------
locs : array_like
ticks : array_like
A list of positions at which ticks should be placed. You can pass an
empty list to disable xticks.

Expand Down Expand Up @@ -1427,36 +1427,34 @@ def xticks(*args, **kwargs):
"""
ax = gca()

if len(args) == 0:
if ticks is None and labels is None:
locs = ax.get_xticks()
labels = ax.get_xticklabels()
elif len(args) == 1:
locs = ax.set_xticks(args[0])
elif labels is None:
locs = ax.set_xticks(ticks)
labels = ax.get_xticklabels()
elif len(args) == 2:
locs = ax.set_xticks(args[0])
labels = ax.set_xticklabels(args[1], **kwargs)
else:
raise TypeError('Illegal number of arguments to xticks')
locs = ax.set_xticks(ticks)
labels = ax.set_xticklabels(labels, **kwargs)
for l in labels:
l.update(kwargs)

return locs, silent_list('Text xticklabel', labels)


def yticks(*args, **kwargs):
def yticks(ticks=None, labels=None, **kwargs):
"""
Get or set the current tick locations and labels of the y-axis.

Call signatures::

locs, labels = yticks() # Get locations and labels

yticks(locs, [labels], **kwargs) # Set locations and labels
yticks(ticks, [labels], **kwargs) # Set locations and labels

Parameters
----------
locs : array_like
ticks : array_like
A list of positions at which ticks should be placed. You can pass an
empty list to disable yticks.

Expand Down Expand Up @@ -1506,17 +1504,15 @@ def yticks(*args, **kwargs):
"""
ax = gca()

if len(args) == 0:
if ticks is None and labels is None:
locs = ax.get_yticks()
labels = ax.get_yticklabels()
elif len(args) == 1:
locs = ax.set_yticks(args[0])
elif labels is None:
locs = ax.set_yticks(ticks)
labels = ax.get_yticklabels()
elif len(args) == 2:
locs = ax.set_yticks(args[0])
labels = ax.set_yticklabels(args[1], **kwargs)
else:
raise TypeError('Illegal number of arguments to yticks')
locs = ax.set_yticks(ticks)
labels = ax.set_yticklabels(labels, **kwargs)
for l in labels:
l.update(kwargs)

Expand Down
Loading