Disable certain messages by default #3512
Comments
|
Great issue, I think we could add a column for "remove entirely". For example, I think that everything regarding formatting that can be done by black could be safely removed from the codebase. Black fix it automatically, are we ever going to want to report errors that can be fixed automatically by a tool? Someone that does not know about black could take a painfully long time to apply pep8 manually. At the very least we should tell users to install black to do it (maybe in the score to not spam it in every message). Also, everything that has false-positive should really be disabled until it's fixed, because it makes adoption harder for most people and because Pylint is not supposed to be a linter that annoys you :) I very much agree with the comment on the ticket you linked that we should create multiple configuration profiles. Maybe even disable message when we detect the presence of installed package (isort, flake8 and black, maybe others ?) |
|
The automatic disabling based on what's installed is a good idea. I like that. We should implement a way of disabling that behaviour but that should be easy enough to do. |
|
Hey @AWhetter This is a great list, thank you so much for working on it! Here are some of my notes regarding checks that could be disabled by default or removed entirely:
To answer @Pierre-Sassoulas I don't believe we should remove the formatting category wholesale regardless if the community aggregated around the use of In the long term, a stages solution similar to the one suggested in #746 is definitely my preferred solution, but for now disabling or removing some of these checks works as well. |
When the framework expects a class to modify a base behavior (like Django for model's Metaclass) you can't do that. Or see this answer on Stack with 30+ upvote that tells you to disable I might have got a little carried away with removing all the formatting messages. Probably not everyone uses black+flake8+isort :) Also, I agree that the stages gamify the lint even more and that's probably a good thing to be able to do. But I also like the standard style proposed here. Because in an IDE you won't be able to do 4 steps (Unless pylint switch stage alone ? It would be confusing to have errors appear. Just imagine having a junior create a "core" error in order to make the next stage errors disappear.). That would probably mean pylint is a lot harder to integrate and understand. I read a blog article aimed toward beginner that said, and I paraphrase, "pylint is useless without configuration, so just use flake8". To address this I think pylint should be easier to use for first-time users or if you don't even know you're using pylint because it's integrated in your IDE. The very knowledgable users are already tailoring it to their need so we can just keep taking their So being able to be able to say that you want to follow the "well-known company" style and have pylint behave accordingly would be a huge simplification. I also think the stages are great when you launch pylint yourself, but will be very hard to integrate in an IDE. |
|
@PCManticore Thanks for checking through the list. I've incorporated some of your changes:
I agree with this whole heartedly. |
|
This list includes errors from flake8-docstrings and flake8-import-order so it should probably denote those and also add error codes from pep8-naming |
|
In addition having an automatic checker that turns off duplicate errors from other install checkers would be awesome but in the meantime, just a page in the docs that shows which to turn off with each checker would be supremely helpful. |
|
Just to add to the discussion as to a sane / useful default configuration. My current config has led to very few false positives and make pylint a joy for me. --disable=design, format, bare-except, unneeded-not, undefined-variable, unused-import, unused-variable, missing-module-docstring, missing-class-docstring, missing-function-docstring, no-self-argument, invalid-name, bad-classmethod-argument, bad-mcs-classmethod-argument Design section is false positives galore for me.... so disabling that got rid of most annoyances of pylint Since using this config, pylint has close to 100% rate of helpful and useful messages. |
|
It looks like all of the design section in the above table by @AWhetter was marked as "should be disabled by default". Would that be a welcome PR? Looks like a good first issue for me in pylint. Or does more discussion need to happen slash I'm jumping the gun? |
|
I think a PR would be welcome, there seems to be a lot of messages where everyone agrees that it should be disabled by default. Thank you for being willing to contribute to Pylint :) |
|
@Pierre-Sassoulas I don't think we reached a consensus on disabling the design checkers just yet. There are still valuable checks in that category that ought to be enabled by default, and the most contentious ones were |
|
@PCManticore No worries, I made a PR for if and when a consensus is reached on disabling the design checker. Which design messages do you think merit being enabled by default? From what I've seen most people who are using them change the default parameters for pretty much all of them. If the messages are that individualistic, I think it makes an argument for them to be enabled on an individual basis. |
|
Following up on this issue. I noticed that in the OP by @AWhetter all the design checker messages are recommend to be disabled by default. I created a PR to disable the category by default #3650 since I find pylint to be infinitely more sane this way. @Pierre-Sassoulas approved my PR but wanted another reviewer before merging. @PCManticore chimed in and didn't think we had a consensus to disable design by default yet. So to spur some more conversation along these lines: Which of the design checks does anyone find should remain enabled? (I can look to modifying my PR to include those). The messages in questions are: too-few-public-methods, too-many-public-methods, too-many-return-statements, too-many-branches, too-many-arguments, too-many-locals, too-many-statements (all are refactoring related checks) |
|
I might have been unclear, I think too-many-return-statements, too-many-branches, too-many-arguments, too-many-locals, too-many-statements are all useful indication of high level design flaw (maybe encompassed by the Mc Cabe complexity metric though), too-many-public-methods as well. Only, too-few-public-methods is unavoidable if you use inheritance a lot, or if you use django (Meta in Model or Form) and I think this one should be disabled. |
|
Would you say that these checks are highly individualistic depending on the project? It seems that it isn't possible to have good defaults since how many is too-many or too-few varies so much project to project. If a user is going to configure these defaults, they would also be able to enable them. However, for someone who is looking for a pip-and-play option, these messages can be a nuisance. My view is without proper individual configuration these checks ought to be disabled. |
|
I just found this issue while looking for something else and I just wanted to chime in a bit. Ages ago I wrote prospector to wrap around all the various other tools, mostly pylint but including mccabe and flake8 and so on and so on. One of the things I added was 'profiles' which was designed to do two things:
Similarly, because prospector ran many tools with overlapping functionality, I ended up creating a 'blender' to try to piece together when two or more tools were giving the same error message. The list of errors and codes from tools representing the same thing is in this file: https://github.com/PyCQA/prospector/blob/master/prospector/blender_combinations.yaml . I'm not sure if this is still up to date but it might serve as a useful starting point. Prospector has been slowly adding bugfixes and keeping up with the tools it depends on, but hasn't been getting the new features it did when I started writing it. Having said that, pylint and others have adapted the 'quality of life' features I built in to it so there's not as much need any more. I bring this up as I started putting in several of the concepts in this issue into prospector a while ago, and so there may be useful things to look at and pick through in case there's any useful things. |
Pierre-Sassoulas
added
the
Maintenance
I think we're mostly on the same page, and I agree enable/disable is currently well understood. I'm just trying to express an agnosticism about whether the future "templating" issue has to be extends (i.e. has to be thought of enable/disable) or could be overwriting (and thus thought of as something else). I think it could go either way. However, if we make this equivalence in terminology right now: then that "locks us in" to extending rather than overwriting, because I completely agree that enable/disable is currently well understood and shouldn't change. Forgive me if I'm repeating the obvious, I'm just trying to make sure I understand you. If you have The Washington Post's pylint template in your pylintrc but then use El País' pylint template at the command line, should that overwrite or extend? I'm not sure. Daniel doesn't sound sure either. I think if we call the templates "enable/disable templates" there's our answer--it has to extend. But I don't think it would violate "least surprise" if the El País template overwrote the Washington Post template. It's not a jinja2 template, it's more like "choose your starting point". (Hats off though to the person who decides to create nested pylint templates 😛 ) |
Haha yes, also there's the per-directory configuration that is a whole project in itself and just another layer of extending/overwriting the conf where we'll have to take the same kind of decision (yet another step between CLI message control and per file message control). I'm not suggesting to do the whole template feature for this, but loading something resembling a pylintrc and reusing disable/enable function (probably refactoring them for this) would prevent technical debt/complexity. I can do a MR in this sense if you want. |
Pierre-Sassoulas
added
Needs decision 🔒
I'd warn against any temporary functionality that will be superseded by full templating ability. That just adds more to deprecate at some point. |
Pierre-Sassoulas
added
Needs specification 🔐
pylint apparently removed the bad-whitespace check at some point [1] This adds pycodestyle back to the mix to check for it. [1] pylint-dev/pylint#3512
|
Hi everyone and thanks for the hard work! Sorry for the noise, but what's the status on this? The thread is getting a bit hard to follow and I wonder if some parts of it are ready to be tested, or if users are currently still expected to configure the relaxed settings themselves? Thanks in advance! |
|
We made no-self-use an extension in 2.14 (?), but otherwise this is still a work in progress. There's an actionable list here: #3512 (comment), but we discussed a lot after that, for duplicate code, what to move into extension or not... We probably need a maintainer to summarize what need to be done if we want to make this actionable again. There's also been evolution in what pylint can do (auto upgrade of config would be nice to have before #9088, #5462 ) |
Following on from the discussion on #746, this is a draft proposal to start disabling certain messages by default. Furthermore, to make it easier to run pylint side by side with flake8, we want to document which messages are duplicated in flake8 and possible make them easier to disable as a group.
The following table contains every pylint message excluding those from plugins and py3k checks. For each message it is noted whether it is a style check or a functional check (by functional I mean it validates some sort of run time behaviour of the code being linted or checks for programmer mistakes) or other (when it's technically a style check but I don't think we would want to disable it when a user opts out of style checks, or if the message doesn't fit in the other categories), whether it duplicates a message in flake8, and my opinion on whether I think it should be disabled by default so that we can use it as a discussion point. I normally prefer the linter to be quite strict so it might be worth gathering user feedback on this. Another option would be to give messages levels of strictness and you pick a level to lint at.
The text was updated successfully, but these errors were encountered: