-
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
Support rules from flake8-force-keyword-arguments #3269
Comments
We haven't defined rigorous criteria for including rules in Ruff yet (#2257), but this rule does look a little niche just based on stars and usage, and I think it might be hard to enforce rigorously right now since we can't inspect function signatures across modules. So I'd vote to pass on this one for the moment just given the tradeoff in utility vs. complexity. |
Thanks for the response! I think it would be partially possible with only local information:
For the first rule I think only local information is needed? An autofix could potentially be provided that adds With only local information rule 2 would falsely flag function calls of functions with >X positional-only parameters or with *args. I would personally be fine with manually adding noqa comments there but I can imagine that this disqualifies it. Would rule 1 be an option? |
@charliermarsh I think you made a mistake when you filed pylint-dev/pylint#8667 and therefore PLR0913. It’s much more useful in Python to restrict the number of positional arguments in function definitions. In response to your change, a rule for only positional arguments has been requested for Pylint for that reason: pylint-dev/pylint#9099 |
I would love this. |
|
Aha, that's great, thx! I struggled a bit to get this working, is there a tiny bug in the documentation? |
Thanks! It must be a typo. Do you want to open a PR fixing the docs? The type is here: ruff/crates/ruff_workspace/src/options.rs Line 2766 in 467c091
|
Thx for the opportunity. |
I think this issue can be closed then, since it's possible to achieve the desired behavior with [tool.ruff.lint.pylint]
max-positional-args = 1 |
@Riezebos are you sure this is closed?
AFAICT PLR0917 checks function definitions only (Rule 1). I don't think a check for function calls exists? (Rule 2) |
@jack-mcivor you're right, I guess it is partially solved. I am not a contributor, and I am not the only one interested in these rules. So I don't think it is up to me anymore to decide whether this is open or closed.
Just to make the reference circle complete, this topic was also discusses in #8137 |
I would be very happy to see a rule that flags any function call that uses >X positional arguments |
Related, what would be super useful is some rule that would ensure that arguments are explicitly specified in keyword (rather than positional) format when calling a function that accepts keyword arguments. The purpose here is that if the order or number of arguments in the function signature is ever updated, the user's code would remain unaffected. This would, however, require inspection of each function signature, including those of dependencies. This might not be viable in the short-term, but I wanted to throw out additional motivation beyond "it just looks nicer." |
I would also really like Rule 2 as it makes this much easier to roll out to an existing codebase. If I turn on Rule 1 and someone touches a large file they will have to go add '*' to every large function definition in that file and then change every caller of those functions which is a large amount of work. Just requiring function calls to be keyword if passing over a certain number of args is much more palatable to introduce change for a team. And an autofix would be fantastic. |
Calling a function with a lot of unnamed arguments is a lot less readable than using keyword arguments. These rules enforce keyword arguments if a function has a lot of parameters, which should lead to more readable code. I believe adding them to ruff would be a good idea!
https://github.com/isac322/flake8-force-keyword-arguments
If there is anything I can do for this as a Python dev with practically 0 Rust experience, I would be happy to help.
The text was updated successfully, but these errors were encountered: