-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Cleaning up variable argument signatures #9912
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
Comments
@timhoffm Just tagged the ticket with a bird call for @tacaswell and the other core devs 😈, so you should receive attention ;). |
Not a core dev, but... For your two cases, I think "yes" to locally resolved arguments and "no" to ones that are passed through. I just took a quick look through matplotlib/lib/matplotlib/axes/_axes.py Line 1244 in 85e124a
I imagine cases like matplotlib/lib/matplotlib/pyplot.py Line 1283 in 85e124a
I agree the downstream code in the case of passthroughs should disallow illegal inputs, and I think in most cases it does, though perhaps we don't bother checking for extra useless kwargs all the time. We always work off master branch. I've been active for a few months and only seen one person base a PR off another branch ;-) |
This is definitely not bug fixes so master branch. For functions / methods which are the bottom of the a call stack should ensure that they consume everything, but for practical reasons am wary of blowing up all of the instance of I would also hold off on making anything that isn't already positional, positional (When we get to drop python2 we can make things currently burried in kwargs key-word only 😄 ). That is extending the API in a more brittle direction. (the above is mostly just re-saying what @jklymak said) |
Thanks for your comments. I think it's a reasonable strategy, not to replicate full signatures. Nevertheless, I would like to propose one special case for discussion: Positional arguments of
|
@tacaswell @jklymak I would like to hear your view on the above suggestion concerning positional arguments. Thanks. |
My opinion doesn't count too much, largely because I'm not very much of a pythonista. But I agree that a couple of repeated positional arguments is entirely reasonable, and it makes the pyplot routine's docstrings more accessible. |
This issue should be on the v3.0 milestone!
This will be really nice for anyone who treats function signatures as usage hints - which includes many users of docs, IPython and Jupyter, etc. I'm certainly excited!
Clever idea that might help with this:
Somebody ping me when there's a branch that doesn't support Python 2, and I'd be happy to write the patch 😄 |
@Zac-HD You can start writing that against master, just won't be merged until we tag 2.2 (later this month). How much over-head would such a decorator add? We have some messy things that may pass to more than one and in many cases, the final consumer of kwargs is the |
Idea update: IMO a decorator or similar just isn't worth the effort; any large-scale editing effort would be better off fixing general style and structure issues. I've done a chunk of manual cleanup and opened a PR instead 😄 I plan to do a short series of these, on the basis that incremental improvements that get merged are better than big changes which never turn up! Hey, how many places (as of 17b6c30) do you think are using
What other searches might be useful? Note that "useful" here really means "finds something we should fix, with low false-positive rate" 😉 For example, it's idiomatic to use sequences directly in a boolean context, ie
In the course of writing out the argspec for This should probably be treated as a bug, and the behaviour for y changed to match x. |
There are a lot of
*args, **kwargs
signatures in the API. In many cases, they could be better described by explicit arguments/kw-arguments. This makes for a more clear API and reduces the chance of unintended misuse.For example pyplot.delaxes
would be more clear with
For all intended uses, the API compatibility is preserved. Unreasonable and possibly wrong uses like
delaxes(ax1, ax2)
ordelaxes(ax, mykwarg=True)
would raise an Exception after the change. In itself, this is a benefit, because these cases will then be detected as false usage. However it may break existing programs.Before I do any pull requests, is there a policy how to handle such cases?
pyplot.savefig
?The text was updated successfully, but these errors were encountered: