Extra arg for "method" (self) and "classmethod" (cls) #10443
Conversation
This comment has been minimized.
This comment has been minimized.
|
Hey @fgallaire thank you for contributing to pylint. You can launch the tests locally with pytest, there's more information in the contributor guide : https://pylint.readthedocs.io/en/stable/development_guide/contributor_guide/index.html |
Hello @Pierre-Sassoulas thanks for the feedback, just coded it as I was confronted to the problem (convenient function with 5 parameters for a class with an I will try to fix the commit. |
|
Looks good. I believe there is an open issue for this. Can we find and link it? #8675 I think? |
mbyrnepr2
added
Bug πͺ²
False Positive π¦
|
Hi @jacobtylerwalls #8675 issue, which I didn't even know existed, perfectly explains what I wanted to fix when I first ran Pylint on my new project. Reassured to see that other people had the same understanding of this behavior as me. Just surprised it hasn't been fixed for a long time! |
Codecov ReportAll modified and coverable lines are covered by tests β
Additional details and impacted files@@ Coverage Diff @@
## main #10443 +/- ##
=======================================
Coverage 95.88% 95.88%
=======================================
Files 176 176
Lines 19140 19145 +5
=======================================
+ Hits 18352 18357 +5
Misses 788 788
π New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Thanks for this!
-
The files tab shows "unchanged files with check annotations" -- can you fix those here by removing the pylint disables?
-
I'd expect to see here new test cases using
@staticmethodand@classmethod. (edit: I see Pierre mentioned this already!) -
Please change the suffix of your news fragment file to
false_positive-- the tooling for compiling release notes depends on this.
|
@Pierre-Sassoulas The primer result seems to show new false negative fixes, which we'd usually hold for a minor release. But I can't tell from the diff why they're happening. Hopefully it's not a caching mystery. Do you think we should hold this for 4.0? We should probably also limit the primer to no more than 100 messages per package or 20 per message/package pair, something like that, so we can see results besides just home-assistant. |
|
Let's wait for 4.0, this seem like a big change. The primer is indeed unreadable at the moment. Maybe because it's a big change, but we could definitely make it less verbose and then limit the number of shown message per project when there's a lot. I started doing this at some point but a parallel refactor merged on main is very hard to rebase on. Probably more motivating to start from scratch. but I have a lot going on at the moment. (I'll probably focus on 3.14 in astroid/pylint then #10425) |
|
@fgallaire That doesn't mean we'll slow down the review process at all, we can still merge this without backporting it to the 3.x branches. My hope is to release 4.0 later this year, but we'll see. |
Type of Changes
Description
(non static) method and classmethod max_arg has to be incremented over the function and staticmethod one, to leave self and cls alone.
Closes #8675