-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add rule “too many positional arguments in function definition” #8946
Comments
I'm totally fine to add a rule for this, though ideally it'd be added to Pylint so that we can use the same identifier... Or perhaps @Pierre-Sassoulas, if there's interest from the Pylint side, could reserve an identifier for it? |
We can definitely create the msgid / symbol before someone implement it. |
## Summary Adds a rule that bans too many positional (i.e. not keyword-only) parameters in function definitions. Fixes #8946 Rule ID code taken from pylint-dev/pylint#9278 ## Test Plan 1. fixtures file checking multiple OKs/fails 2. parametrized test file
The pylint docu pointed me into this issue. |
It’s pretty simple: Good APIs don’t have many positional parameters. Most functions take 1–2 “main objects” to operate on and a few parameters that modify how they operate, e.g. There are few exceptions where 4 or more positional parameters make sense, e.g. |
But what is the difference compared to |
positional vs total arguments. you can configure:
Then this would be legal: def (a, b, c, *, x, y, z): ... and this wouldn‘t (because while it has <=6 arguments, it has >3 positional ones) def (a, b, c, x, y, z): ... |
I played a bit around in my REPL. def foobar(a, b, c, *, x, y, z): ... Do I get this correct? The In other words the "error" EDIT:
This would force the caller to give every argument a name when calling the function? |
Thank you very much with contributing to my current and future FOSS projects with teaching me such an essential Python feature. Was new to me. |
Always happy to do so! |
@buhtz did you read pylint's 3.3.1's doc or the latest ? And what helped you understand the concept so we can put it in the doc ? Sorry for not removing the link to this issue, it was relevant at a time when the pylint check was not implemented yet, but not anymore. |
Not sure which version it was. But for myself I would say both versions you linked to wouldn't have helped me much. But he problem is more on my site not on yours. I think I lacked of basic (?) python concepts. I am not sure if a pylint-rule-docu should be responsible to explain them. But I would say a link to the Python doc about special parameters would be helpful. And what also would help is to extend the example code. Maybe you can add to the "correct code" section something like this making the effect of
To my experience, as someone reading all the free python learning material and working with Python for nearly 20 years on a hobby level, that "special parameters" are unknown to some of the beginners or hobby Python users. No one reads the full python docu, of course. ;) I do assume that you have to study computer science or book a payed python course to learn things like this. |
Since #4329, PLR0913 is useless to us. We need a rule that doesn’t restrict the number of total arguments, but the number of non-keyword-only arguments. See also pylint-dev/pylint#9099
I’m happy to do a PR, but I don’t know how I should go about the rule identifier: Should I go for a new RUFF rule, or is there a way I can reserve a new code like e.g. PLR0914 in Pylint for this even if I don’t plan to make a PR for Pylint?
The text was updated successfully, but these errors were encountered: