-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Silently Ignored Kwargs Cleanup #18748
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
Cool, yeah would be nice to fix these up. |
certainly on a user facing level we should do this. On internal functions, this is generally a code-simplicity exercise, so generally ok to remain. |
By fix, are we talking remove or implement or both? For instance, I've come across Dataframe.round(), where def round(self, decimals=0, *args, **kwargs): I can envision an API that accepts other inputs (maybe df.round(A=2) instead of df.round({'A': 2}), for instance), but that's a major project compared to just removing Setting that question aside, with simpler changes, should I just create a pull request and commit the easier ones there, linking back to this issue? |
.round something of a special case as it’s done this way for numpy compat we have a whole module on numpy compat for this reason |
I'm going to take a shot at this. Probably going to make several PRs for it, package by package. I'm focusing mainly on the user facing functions. |
Inspired by #18699 I wrote up a very brief script that when placed in the root directory could help identify functions where
**kwargs
are accepted but not used in the method body. While not perfect (may give false positives in cases of nested function definitions) it calls out 128 functions as is (see extra_kwargs.txt). A quick spot check on my end looked good.Sharing in case anyone would like to look through this and submit PR's to tackle in batches. Here's the script I used in case anyone is interested
The text was updated successfully, but these errors were encountered: