Skip to content

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

Closed
WillAyd opened this issue Dec 12, 2017 · 5 comments
Closed

Silently Ignored Kwargs Cleanup #18748

WillAyd opened this issue Dec 12, 2017 · 5 comments
Labels
API Design Clean Error Reporting Incorrect or improved errors from pandas good first issue

Comments

@WillAyd
Copy link
Member

WillAyd commented Dec 12, 2017

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

import re
import glob

if __name__=='__main__':
    pater = re.compile(r"""
    ^(.+?)       # group to match function name (non-greedy)
    \((.*?)\)    # group to match arguments
    (.*)$        # everything else (function body)
    """, flags=re.DOTALL|re.VERBOSE)

    fns = glob.glob('pandas/**/*.py', recursive=True)
    for fn in fns:
        with open(fn) as infile:
            source = infile.read()
            funcs = source.split('def ')
            for func in funcs:
                res = re.search(pater, func)
                if res:
                    nm, args, body = res.groups()
                    if '**kwargs' in args and not 'kwargs' in body:
                        args = re.sub('\s+', ' ', args)
                        print("{} def {}({})".format(fn, nm, " ".join(
                            args.split("\n"))))
@chris-b1 chris-b1 added Clean Difficulty Novice Error Reporting Incorrect or improved errors from pandas API Design labels Dec 12, 2017
@chris-b1
Copy link
Contributor

Cool, yeah would be nice to fix these up.

@jreback
Copy link
Contributor

jreback commented Dec 12, 2017

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.

@jreback jreback added this to the Next Major Release milestone Dec 12, 2017
@NoahTheDuke
Copy link
Contributor

By fix, are we talking remove or implement or both?

For instance, I've come across Dataframe.round(), where args and kwargs aren't allowed to be defined when calling:

    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 args and kwargs. How should I make such a decision?

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?

@jreback
Copy link
Contributor

jreback commented Feb 21, 2018

.round something of a special case as it’s done this way for numpy compat
basically numpy accept some keywords that we don’t in pandas and we want to allow
np.round to work on a dataframe (internally some numpy routines call the function in the object with some passed in kwargs)

we have a whole module on numpy compat for this reason

@kprestel
Copy link
Contributor

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.

@jreback jreback modified the milestones: Contributions Welcome, 0.24.0 Oct 30, 2018
@jreback jreback modified the milestones: 0.24.0, Contributions Welcome Dec 2, 2018
@WillAyd WillAyd closed this as completed Feb 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Clean Error Reporting Incorrect or improved errors from pandas good first issue
Projects
None yet
Development

No branches or pull requests

6 participants