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

R0913 excludes keyword- and positional-only arguments #8667

Closed
charliermarsh opened this issue May 8, 2023 · 5 comments · Fixed by #8674 or #9093
Closed

R0913 excludes keyword- and positional-only arguments #8667

charliermarsh opened this issue May 8, 2023 · 5 comments · Fixed by #8674 or #9093
Labels
False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@charliermarsh
Copy link

Bug description

Pylint's too-many-arguments rule seems to exclude keyword- and positional-only arguments from the "total argument" count:

def f(a, b, c, d, e, *, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t):
    ...


def g(a, b, c, d, e, f):
    ...

Configuration

No response

Command used

pylint a.py

Pylint output

************* Module a
a.py:5:0: R0913: Too many arguments (6/5) (too-many-arguments)

Expected behavior

My instinct is that it should include keyword- and positional-only arguments, but I'm wondering if this is intentional.

Pylint version

pylint 2.17.2
astroid 2.15.3
Python 3.11.1 (main, Dec 23 2022, 09:25:23) [Clang 14.0.0 (clang-1400.0.29.202)]

OS / Environment

No response

Additional dependencies

No response

@charliermarsh charliermarsh added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label May 8, 2023
@mbyrnepr2 mbyrnepr2 added False Negative 🦋 No message is emitted but something is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels May 9, 2023
@mbyrnepr2
Copy link
Member

mbyrnepr2 commented May 9, 2023

Thanks for this @charliermarsh! I believe that the too-many-arguments checker predates positional-only arguments and explains why they were not originally handled by the checker. I don't think there is any reason to exclude the positional-only and keyword-only parameters from the check.

@Pierre-Sassoulas Pierre-Sassoulas added the Needs PR This issue is accepted, sufficiently specified and now needs an implementation label May 9, 2023
mbyrnepr2 added a commit to mbyrnepr2/pylint that referenced this issue May 10, 2023
@Pierre-Sassoulas Pierre-Sassoulas added this to the 3.0.0a7 milestone May 10, 2023
mbyrnepr2 added a commit to mbyrnepr2/pylint that referenced this issue May 11, 2023
Pierre-Sassoulas pushed a commit that referenced this issue May 15, 2023
@parched
Copy link

parched commented May 19, 2023

@charliermarsh @mbyrnepr2 This going to throw up a lot of warnings in my code. I believe the intention of this rule is to ensure args don't get mixed up. That can't happen with kwargs, as they're explicit. I do believe position-only args should be included though.

@Pierre-Sassoulas
Copy link
Member

I think that too-many-arguments is a refactoring check and its intention is only to limit the number of args of a function (i.e. you should create a data structure to group semantically close arguments together). We asked ourselves if we should increase the default value from 5 to 7 though, there's indeed a lot of new message raised. Not having too many positional argument because you don't know what is what is another check not yet implemented (#385)

@haukex
Copy link

haukex commented Oct 2, 2023

I just ran into this on upgrading to 3.0.0 and indeed did have to increase my max-args from 5 to 7. I also agree with @parched's point about this check being useful for avoiding confusion when there are too many positional arguments - excluding keyword arguments from the check would a nice thing to have too.

@sabik
Copy link

sabik commented Oct 3, 2023

Thirded (fourthed?) on the distinction between too many arguments total and too many positional arguments, and submitted as a separate feature request issue #9099

DifferentialOrange added a commit to tarantool/tarantool-python that referenced this issue Oct 3, 2023
Disable newly uncovered cases of R0913 too-many-arguments. We don't plan
to change existing public API now.

Since pylint 3.0.0, issue [1] has been fixed, which uncovered several
new too-many-arguments cases.

1. pylint-dev/pylint#8667
DifferentialOrange added a commit to tarantool/tarantool-python that referenced this issue Oct 3, 2023
Disable newly uncovered cases of R0913 too-many-arguments. We don't plan
to change existing public API now.

Since pylint 3.0.0, issue [1] has been fixed, which uncovered several
new too-many-arguments cases.

1. pylint-dev/pylint#8667
abulenok added a commit to open-atmos/PySDM that referenced this issue Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
6 participants