Bump pylint from 3.2.7 to 3.3.0 #5306
Conversation
![dependabot[bot]](https://avatars.githubusercontent.com/in/29110?s=60&v=4){kind=small}
Bumps [pylint](https://github.com/pylint-dev/pylint) from 3.2.7 to 3.3.0. - [Release notes](https://github.com/pylint-dev/pylint/releases) - [Commits](pylint-dev/pylint@v3.2.7...v3.3.0) --- updated-dependencies: - dependency-name: pylint dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
dependabot
bot
added
dependencies
This makes the current codebase pass with pylint 3.3.0 while still warning in case many positional arguments are used.
pyproject.toml
Outdated
| @@ -202,6 +202,7 @@ disable = [ | |||
| "useless-return", # PLR1711 | |||
| # "no-self-use", # PLR6301 # Optional plugin, not enabled | |||
| ] | |||
| max-positional-arguments = 10 | |||
There was a problem hiding this comment.
this needs to go under 'tool.pylint.DESIGN' like discussed in the matter server conversation
There was a problem hiding this comment.
It does work here too (as you can see from the tests), but agreed, according to https://pylint.pycqa.org/en/latest/user_guide/configuration/all-options.html#max-positional-arguments it is under design. Seems to be "more" correct then? 🤔 🤷
There was a problem hiding this comment.
Tbh if we're going to set it to 10 I'd say just disable it. Core already disables it so we'd be consistent that way. It seems like more of an opinionated one to me anyway.
But I believe the point of this rule is just to force people to switch to named parameters after a small number of positional ones. Because when there is too many positional ones people have to look up the documentation or definition any time they want to figure out what's going on because its too many to remember. 10 is definitely well over that complexity threshold so we are effectively just ignoring this rule with this change already, might as well make it official.
There was a problem hiding this comment.
I guess I put my comment in the existing thread rather then a review, my bad.
Anyway I'm fine with it if we want to marge it. But I think if we set the number as high as 10 we're already defeating the purpose of this rule. If a developer encounters 10 positional parameters that is well past the complexity threshold where it is easy to remember and understand what those do. They'll have to look up documentation and match up argument to doc to see what's going on whereas if it was named parameters they might be able to just read and get it.
So rather then set it that high I'd say just disable the rule. Which is what core does as well.
|
A newer version of pylint exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged. |
Isn't that an argument for the rule? I mean, you support the essential idea of trying to avoid too many arguments. Just because between 5 and 10 is already bad, does not mean we cannot have a rule warning when we are above 10. I just picked 10 since there are already functions with 9 in the codebase, but none have 10 or more today. Sure, this won't address the ones between 5 and 10, but this hopefully will encourage that no function will go beyond 10 😅 In the end, it helps support what pylint intended: Avoid too many parameters. Granted, we are at a higher level (pylint's default is a rather conservative 5), but IMHO, it is better to keep a warning if it potentially helps? 🤷♂️ I'll merge as is, we can still drop in case we don't like the warning. |
Bumps pylint from 3.2.7 to 3.3.0.
Commits
6350dfaBump pylint to 3.3.0, update changelog78f3dfaBump astroid to 3.3.3 (#9939)b28c1f6Add check forunnecessary-default-type-args(#9938)bd97b93[doc framework] Assert that the good and bad example exists in the doc (#9936)7aa4436Fix duplicate workflow step ids (#9934)0950916[pre-commit] Add codespell, and fix some existing typos (#9912)3b4a7f9Add details.rst for c-extension-no-member (#9933)7d60c27Explicitly save cache in primer jobs67acc96Add additional stdlib deprecations (mostly 3.13) (#9853)0adf671Remove old-style classes code, remove check for new-style class (#9925)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)