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

False positive with default arg values before *args #2481

Closed
eirnym opened this issue Sep 8, 2018 · 12 comments
Closed

False positive with default arg values before *args #2481

eirnym opened this issue Sep 8, 2018 · 12 comments

Comments

@eirnym
Copy link

eirnym commented Sep 8, 2018

Steps to reproduce

  1. run pylint on a file with function below
def fun(param1, param2, param3=True, *args):
    """Pylint false positive

        >>> fun(1, 2)
        (1, 2, True, ())
        >>> fun(1, 2, 3)
        (1, 2, 3, ())
        >>> fun(1, 2, 3, 4)
        (1, 2, 3, (4,))
    """
    return (param1, param2, param3, args)

Current behavior

W1113: Keyword argument before variable positional arguments list in the definition of fun function (keyword-arg-before-vararg)

Expected behavior

different warning if any as variable param3 is not a keyword argument.

pylint --version output

pylint 2.1.1
astroid 2.0.4
Python 3.7.0 (default, Jun 28 2018, 06:05:58) 
[Clang 9.0.0 (clang-900.0.39.2)]
@PCManticore
Copy link
Contributor

This is expected. The error message for that check basically says:

  When defining a keyword argument before variable positional arguments, one can
  end up in having multiple values passed for the aforementioned parameter in
  case the method is called with keyword arguments. This message belongs to the
  typecheck checker.

Which means that for your function you can't really pass param as keyword argument and pass variadic positional arguments as well:

$ python a.py
Traceback (most recent call last):
  File "a.py", line 13, in <module>
    fun(2, 3, 4, param=4)
TypeError: fun() got multiple values for argument 'param'

@eirnym
Copy link
Author

eirnym commented Sep 9, 2018

@PCManticore This is not a /keyword/ argument. this is just an argument with default value.

And an error message occurs when I define a function, not when I pass params.

@PCManticore
Copy link
Contributor

@eirnym We don't make any distinction between positional or default and keyword only for this check. The result of your function signature is that you can never pass both variadic (*args ) and param as a keyword argument, which exactly what we are emitting here. It doesn't matter if the check is not emitted when you call this function, since the problem starts with the function definition by itself, not with any potential call.

@eirnym
Copy link
Author

eirnym commented Sep 9, 2018

Specification of keyword argument before positional arguments is a syntax error. And you aren't able pass a keyword argument with the same name as positional anyway.

I see a few problems with combining all cases in the warning:

  • Manually taking such arguments from *args is a bit of code and more potentional mistakes, and Python VM will do it automatically and it's well-tested
  • The case fun(2, 3, 4, param=4) is covered even in Python tutorial, and I'd probably want to have an error from static analyzer only on the place I call, not where I define it.
  • Hiding it function-wise, analyzer hide it from all calls to it, so I don't want to do it as well.

Therefore, I want an explicit distinction between these two cases.

@CoinCheung
Copy link

@PCManticore
Hi, do you mean that, if I want to do it correctly, I should write code like this: def fun(param1, param2, *args, param3=True, **kwargs): pass ?

@eirnym
Copy link
Author

eirnym commented Apr 1, 2019

@CoinCheung Sometimes it's just not possible to write this way as you wrote, but this is the way he wants.

@CoinCheung
Copy link

CoinCheung commented Apr 2, 2019

@eirnym So does pylint suggests that it is a good habit to never use *args and **kwargs ?

@chuckoy
Copy link

chuckoy commented Apr 4, 2019

@CoinCheung for what it's worth, PEP-3102 uses that convention (i.e.: def compare(a, b, *, key=None):) in its specification. I have always used the format @eirnym first posted as that's what just naturally came to me (edit: I've not used this style of param signature yet, I was thinking of def compare(a, b, *args, **kwargs)).

I don't think pylint suggests it's a good habit to never use *args and **kwargs, but rather if you do use it, to put *args at the end of positional arguments and just before keyword arguments. When I changed my code to follow this, the warning disappeared 🙂

In the end, though, pylint is a linter that you configure. If you decide that this warning isn't relevant to you and your project, you can disable it in .pylintrc.

(Disclaimer: I've only recently discovered pylint and am still in the process of getting familiar with it and setting it up in one of my projects.)

Edit: #1835 notes that that syntax is invalid in Python2 so it seems there's an inherent conflict between the warning and Python2.

@Wuestengecko
Copy link

Wuestengecko commented Mar 20, 2020

Hello, I realize this is by now quite an old issue, however the preconditions have changed a bit.

PEP-570, first implemented in Python 3.8, introduced the ability to define positional-only parameters which cannot be passed in as keywords, using the /:

def fun(param1, param2, param3=True, /, *args):

Defined like this, there is no way for param3 to have an ambiguous value, because it will always be populated from the positional parameters. If param3=False were to be passed into fun, it would either be put into a **kwargs or — as there is none in this case — rejected with:

TypeError: fun() got some positional-only arguments passed as keyword arguments: 'param3'

Pylint 2.4.4 does not seem to know about this and still raises a "keyword-arg-before-vararg" warning on the above definition.

The / in the parameter list is a syntax error in Python 3.7 and older. I believe it would be best not actively recommending its use until 3.8 has sufficiently propagated through slower-rolling distributions like Debian (currently 3.7) or CentOS (3.6). However, for projects targetting 3.8+ the syntax should IMO already be accepted.

@RaenonX
Copy link

RaenonX commented Aug 23, 2020

Still hoping the pylint would be able to recognize PEP-570. Seems that this syntax is not yet recognized.

My current Pylint version

pylint 2.5.3
astroid 2.4.1
Python 3.8.5 (tags/v3.8.5:580fbb0, Jul 20 2020, 15:57:54) [MSC v.1924 64 bit (AMD64)]

@fievelk
Copy link

fievelk commented Apr 7, 2021

Hi, it looks like this is still not fixed (pylint 2.7.4). Is there a reason for this? Will it ever be able to recognize PEP-570?

@Pierre-Sassoulas
Copy link
Member

@fievelk The issue is closed and about another problem in another context. Please open a new issue describing the problem in the context of PEP570, and consider contributing if this issue is important for you. pylint will be able to recognize pep-570 the day someone has the incentive to implement it, and actually does.

@pylint-dev pylint-dev locked as off-topic and limited conversation to collaborators Apr 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants