Skip to content
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

keyword-arg-before-vararg should not be raised with python2 #1835

Closed
jsmedmar opened this issue Jan 12, 2018 · 4 comments

Comments

@jsmedmar
Copy link

commented Jan 12, 2018

Steps to reproduce

  1. In python 2 define a function like:
def test(a, b=1, c=2, *args, **kwargs):
    print(args)
  1. With pylint 1.8.1 you will get the following error.
Keyword argument before variable positional arguments list in the definition of test function (keyword-arg-before-vararg)
  1. However, the solution is a python2 SyntaxError:
def test(a, *args, b=1, c=2, **kwargs):
    print(args)

Current behavior

As per 1.8.1 release notes:

A new check was added, keyword-arg-before-vararg.
This warning message is emitted when a function is defined with a keyword argument appearing before variable-length positional arguments (*args). This may lead to args list getting modified if keyword argument's value is not provided in the function call assuming it will take default value provided in the definition.

Expected behavior

I think this warning shouldn't be raised in python2.

pylint --version output

pylint --version
Using config file /ifs/work/leukgen/home/s/repos/toil_battenberg/.pylintrc
pylint 1.8.1, 
astroid 1.6.0
Python 2.7.11 (default, Jan 11 2016, 13:50:06) 
[GCC 4.4.7 20120313 (Red Hat 4.4.7-16)]

jsmedmar added a commit to papaemmelab/cookiecutter-toil that referenced this issue Jan 12, 2018

🔥 feat: improve pylint version to 1.8.1
Wanted to improve pylint to latest version so that
we all were up to date.

Had to ignore 2 errors in `.pylintrc`:

    superfluous-parens,
    keyword-arg-before-vararg,

Because they don't play nice with python 2.
`superfluous-parens` gets raised when using print(),
whilst `keyword-arg-before-vararg`'s solution results
in a python2 `SyntaxError`; see this
[ticket](PyCQA/pylint#1835).
@PCManticore

This comment has been minimized.

Copy link
Member

commented Jan 18, 2018

While the solution is a syntax error in Python 2, the code is still faulty as per the description of the message. There will be no way to pass b as keyword argument and pass extra positional args as well. IMO, this function signature should be just changed.

@jsmedmar

This comment has been minimized.

Copy link
Author

commented Jan 30, 2018

@PCManticore hey thanks so much! you are right

pomegranited added a commit to open-craft/XBlock that referenced this issue Mar 12, 2018

Fix quality failures
* keyword-arg-before-vararg: cf PyCQA/pylint#1835
* inconsistent-return-statements
@MrMino

This comment has been minimized.

Copy link

commented May 18, 2018

@PCManticore what if I want the b and c parameters to have default values, but I also want the function to take arbitrary number of arguments? E.g. whenever I need these arguments, because a decorator that uses that definition requires it?

@AWhetter

This comment has been minimized.

Copy link
Contributor

commented May 18, 2018

It's not a great solution because the defaults aren't included in the function signature but I would do the following in this case:

kwargs.setdefault('b', 1)
kwargs.setdefault('c', 2)

It's not ideal, but mixing keyword arguments and variable arguments can get messy really quickly. So I think that using setdefault is a better solution, especially if you document the new default value in the docstring.

This is a matter of opinion though so feel free to add a new option to cover this case. The 1.9 branch is where the Python 2 compatible version exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.