-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add decorator to inherit keyword-only deprecations #14130
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
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).
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. |
45901b1
to
5558749
Compare
Thanks for the feedback Understanding what this does
is more complicated than
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 |
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.) |
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. I'm really not sure which way is better. This should be discussed with the other developers. |
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 for 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. |
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 parameter |
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.py
index 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.py
index 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.py
index 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) |
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 |
I personally think the fake-body approach is cleaner, but let's see what others think. |
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", lambda new: None).__code__,
cbook._make_keyword_only:
cbook._make_keyword_only("", "p", lambda p: None).__code__,
}
def _wraps(method, func=None):
if func is None:
return functools.partial(_wraps, method)
decorators = [docstring.copy(method)]
while getattr(method, "__wrapped__", None) is not None:
for decorator_maker, code in _code_objs.items():
if method.__code__ is code:
kwargs = {
k: v.cell_contents
for k, v in zip(code.co_freevars, method.__closure__)}
assert kwargs["func"] is method.__wrapped__
kwargs.pop("func")
decorators.append(decorator_maker(**kwargs))
method = method.__wrapped__
for decorator in decorators[::-1]:
func = decorator(func)
return func diff --git i/tools/boilerplate.py w/tools/boilerplate.py
index 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} which I guess is not too different from your solution. |
@anntzer Not following all the details, but the concept looks good! Want to make a PR? 😄 |
sure, #15254 |
Closing in favor of #15254. |
PR Summary
This PR allows to deprecate positional usage of arguments also for Axes/pyplot functions.
Motivation
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: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 to
Axes.set_xlabel
:The new
_inherit_make_keyword_only
decorator reads these parameters and thus can create a wrapper for the respective pyplot function:which is similar to
but with two advantages:
set_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