This PR enables a straight forward way to deprecate positional usage of arguments so that we can cleanly evolve the API to a state in which only a few positional parameters are allowed per function (e.g.def set_xlabel(self, xlabel, *, fontdict=None, ...). This restriction of the API has two goals:

Prevent easily-breaking use by passing lots of positional arguments (don't allow bad style).
It gives us more freedom to change the kwarg-only parts of the signatures; e.g. removing or introducing parameters.

How it works

This augments the_make_keyword_only decorator, which couldn't be used with the above functions so far. It works like this:

The_make_keyword_only decorator attaches its call parameters to the wrapper function.
This decorator can e.g. be applied toAxes.set_xlabel:
```
@_make_keyword_only('fontdict')def set_xlabel(self, xlabel, fontdict=None, labelpad=None, **kwargs):
```
The new_inherit_make_keyword_only decorator reads these parameters and thus can create a wrapper for the respective pyplot function:
```
@_inherit_make_keyword_only(Axes.set_xlabel):def xlabel(xlabel, fontdict=None, labelpad=None, **kwargs):
```
which is similar to
```
@_make_keyword_only('fontdict')def xlabel(xlabel, fontdict=None, labelpad=None, **kwargs):
```
but with two advantages:
- We don't have to explicitly state 'fontdict' again.
- The wrapper additionally suppresses warnings from the called function (wrapper ofset_xlabel).
boilerplate.py now applies the_inherit_make_keyword_only decorator to all generated pyplot functions. Manually written wrappers would have to add it themselves if needed.
There's one additional detail to the pyplot wrappers:boilerplate.py generates their signature based on the signature of the called (e.g. Axes-) function. Now, we still have to take the original signature, not the signature that was rewritten by_make_keyword_only(Axes.set_xlabel), otherwise we cannot apply_make_keyword_only to the pyplot function.

Scope

This is only the technical implementation plus tests. Actual_make_keyword_only depreactions will be handled and discussed in separate PRs.

PR Checklist

Has Pytest style unit tests
Code isFlake 8 compliant

timhoffm added the API: argument checking label

May 4, 2019

timhoffm added this to thev3.2.0 milestone

May 4, 2019

Copy link

Contributor

anntzer commentedMay 5, 2019

If we want to support this, it may be easier to use a decorator to completely swap out the body of the pyplot wrapper; something like (approximately, untested).

@pyplot_wrapper(gca, Axes.foo, ret_handler=lambda ret: ...)def foo(...):    pass  # see pyplot_wrapper for the actual implementationdef pyplot_wrapper(obj_getter, unbound_method, ret_handler=lambda x: x, func=None):    if func is None: return partial(...)    @docstring.copy(unbound_method)    @functools.wraps(func)    def wrapper(*args, **kwargs):        ret = unbound_method(obj_getter(), *args, **kwargs)        ret_handler(ret)        return ret    return wrapper

now the "body" of the pyplot function is empty, its signature is just here for static analysis tools, and the actual implementation just forwards args and kwargs "as is" to the wrapped function.

This avoids requiring boilerplate.py to have to know about the deprecating decorators.

Add decorator to inherit keyword-only deprecations

5558749

timhoffm force-pushed themake_keyword_only-pyplot branch from45901b1 to5558749Compare

May 9, 2019 23:27

Copy link

MemberAuthor

timhoffm commentedMay 9, 2019

Thanks for the feedback@pyplot_wrapper is an interesting alternative. I'm a bit concerned that it's too much magic.

Understanding what this does

@pyplot_wrapper(gca, Axes.foo, ret_handler=lambda ret: ...)def foo(...):    pass  # see pyplot_wrapper for the actual implementation

is more complicated than

@cbook._inherit_make_keyword_only(Axes.foo)@docstring.copy(Axes.foo)def foo(...):    return gca().foo(...)

Ok, we have two decorators, but they only perform some sort of preprocessing. The actual execution logic is written out explicitly. I've often investigated what the code actually does by checking the code link in the documentation or?? in ipython. I assume that the second form is more helpful for users trying to understand how pyplot works.

Copy link

Contributor

anntzer commentedMay 10, 2019

Well, we can "keep" the old function body in the source of pyplot.py, it's just that it's not going to be executed :) (then yes, the function body becomes a "lie", but only in the sense that we keep track of what arguments were passed positionally and what arguments were passed by keyword.)

Copy link

MemberAuthor

timhoffm commentedMay 11, 2019

I don't think we should add a "lie" to our code. At some point, the written code could deviate from the actual functionality of the decorator.pass # see pyplot_wrapper for the actual implementation at least clearly states what's going on.

I'm really not sure which way is better. This should be discussed with the other developers.

Copy link

Contributor

anntzer commentedMay 19, 2019•
edited
Loading

I guess another question is, what are the cases where you would want to make a parameter keyword-only?

The case where I introduced this decorator is forpyplot.show(block=...), because I would like to change the firstpositional argument to be a list of figures to show instead. Indeed, the decorator helps when you want to replace a given positional argument by another one. But this should be quite rare. If we want to remove a keyword parameter, we have_delete_parameter (which has the nice side effect of turning the subsequent parameters keyword-only, but that's really just a side effect); if we want to add a keyword parameter, we can add it at the end of the signature. I also don't think that just "not allowing bad style" (of passing tons of arguments positionally) is a sufficient reason for the added complexity.

However, I may certainly be missing some other use cases. Thus, it would be nice to see some examples of the places where you intend to use the improved form of this decorator.

Also, another possibility if you want to use this decorator in pyplot is to just write the pyplot wrapper manually, instead of generating it with boilerplate.py.

Copy link

MemberAuthor

timhoffm commentedMay 19, 2019

There are a number of Axes methods that should IMHO only have a limited number or positional parameters. Apart from the bad style, I cannot safely reorder kw parameters to get them in a meaningful order. For example,#14107 introduced the parameterquantiles toviolinplot, which semantically belongs next toshowmedians, but since there is no limit on positional arguments, this is technically a breaking change.

Copy link

Contributor

anntzer commentedMay 19, 2019

I guess I would rather go with manually writing the wrapper in that case (it's not optimal but not the end of the world either...).

diff --git i/lib/matplotlib/axes/_axes.py w/lib/matplotlib/axes/_axes.pyindex 2bae8ff07..7e80921cc 100644--- i/lib/matplotlib/axes/_axes.py+++ w/lib/matplotlib/axes/_axes.py@@ -7903,6 +7903,7 @@ optional.         return im      @_preprocess_data(replace_names=["dataset"])+    @cbook._make_keyword_only("3.2", "vert")     def violinplot(self, dataset, positions=None, vert=True, widths=0.5,                    showmeans=False, showextrema=True, showmedians=False,                    quantiles=None, points=100, bw_method=None):diff --git i/lib/matplotlib/pyplot.py w/lib/matplotlib/pyplot.pyindex 4f266917e..857c39ffe 100644--- i/lib/matplotlib/pyplot.py+++ w/lib/matplotlib/pyplot.py@@ -2317,6 +2317,21 @@ switch_backend(rcParams["backend"]) install_repl_displayhook()+# Move this back to autogen after deprecation period over.+@docstring.copy(Axes.violinplot)+@cbook._make_keyword_only("3.2", "vert")+def violinplot(+        dataset, positions=None, vert=True, widths=0.5,+        showmeans=False, showextrema=True, showmedians=False,+        quantiles=None, points=100, bw_method=None, *, data=None):+    return gca().violinplot(+        dataset, positions=positions, vert=vert, widths=widths,+        showmeans=showmeans, showextrema=showextrema,+        showmedians=showmedians, quantiles=quantiles, points=points,+        bw_method=bw_method, **({"data": data} if data is not None+        else {}))++ ################# REMAINING CONTENT GENERATED BY boilerplate.py ##############@@ -3011,20 +3026,6 @@ def triplot(*args, **kwargs):     return gca().triplot(*args, **kwargs)-# Autogenerated by boilerplate.py.  Do not edit as changes will be lost.-@docstring.copy(Axes.violinplot)-def violinplot(-        dataset, positions=None, vert=True, widths=0.5,-        showmeans=False, showextrema=True, showmedians=False,-        quantiles=None, points=100, bw_method=None, *, data=None):-    return gca().violinplot(-        dataset, positions=positions, vert=vert, widths=widths,-        showmeans=showmeans, showextrema=showextrema,-        showmedians=showmedians, quantiles=quantiles, points=points,-        bw_method=bw_method, **({"data": data} if data is not None-        else {}))-- # Autogenerated by boilerplate.py.  Do not edit as changes will be lost. @docstring.copy(Axes.vlines) def vlines(diff --git i/tools/boilerplate.py w/tools/boilerplate.pyindex caec52474..aaeb60ff4 100644--- i/tools/boilerplate.py+++ w/tools/boilerplate.py@@ -249,7 +249,7 @@ def boilerplate_gen():         'tricontourf',         'tripcolor',         'triplot',-        'violinplot',+        # 'violinplot',         'vlines',         'xcorr',         # pyplot name : real name

(or with the "fake body" approach)

Copy link

MemberAuthor

timhoffm commentedMay 19, 2019

That doesn't quite work. I can only do changes of the order after the deprecation period. Thus, when I realize I want to change something, I would have to wait a year to do that.

The intended way of working would be to add kwonly deprecations now in reasonable positions, switch them to* after the deprecation and be free from then on.

Copy link

Contributor

anntzer commentedMay 19, 2019

I personally think the fake-body approach is cleaner, but let's see what others think.

anntzer mentioned this pull request

Aug 14, 2019

Annotate argument in axes class match upstream#15049

Merged

6 tasks

tacaswell modified the milestones:v3.2.0,v3.3.0

Sep 10, 2019

tacaswell self-requested a review

September 10, 2019 20:01

Copy link

Contributor

anntzer commentedSep 10, 2019

Actually, it is possible to reconstruct the decorators that need to be applied to the pyplot wrappers just from the axes methods... I think the following works:

# in pyplot.py_code_objs= {cbook._rename_parameter:cbook._rename_parameter("","old","new",lambdanew:None).__code__,cbook._make_keyword_only:cbook._make_keyword_only("","p",lambdap:None).__code__,}def_wraps(method,func=None):iffuncisNone:returnfunctools.partial(_wraps,method)decorators= [docstring.copy(method)]whilegetattr(method,"__wrapped__",None)isnotNone:fordecorator_maker,codein_code_objs.items():ifmethod.__code__iscode:kwargs= {k:v.cell_contentsfork,vinzip(code.co_freevars,method.__closure__)}assertkwargs["func"]ismethod.__wrapped__kwargs.pop("func")decorators.append(decorator_maker(**kwargs))method=method.__wrapped__fordecoratorindecorators[::-1]:func=decorator(func)returnfunc

diff --git i/tools/boilerplate.py w/tools/boilerplate.pyindex caec52474..74f4a72b2 100644--- i/tools/boilerplate.py+++ w/tools/boilerplate.py@@ -36,7 +36,7 @@ AUTOGEN_MSG = """ # Autogenerated by boilerplate.py.  Do not edit as changes will be lost."""  AXES_CMAPPABLE_METHOD_TEMPLATE = AUTOGEN_MSG + """-@docstring.copy(Axes.{called_name})+@_wraps(Axes.{called_name}) def {name}{signature}:     __ret = gca().{called_name}{call}     {sci_command}@@ -44,13 +44,13 @@ def {name}{signature}: """  AXES_METHOD_TEMPLATE = AUTOGEN_MSG + """-@docstring.copy(Axes.{called_name})+@_wraps(Axes.{called_name}) def {name}{signature}:     return gca().{called_name}{call} """  FIGURE_METHOD_TEMPLATE = AUTOGEN_MSG + """-@docstring.copy(Figure.{called_name})+@_wraps(Figure.{called_name}) def {name}{signature}:     return gcf().{called_name}{call}