Closed PCManticore closed 2 years ago
RonnyPfannschmidt
commented
8 years ago what i want from a static analysis tool/linter as a normal developer day to day is simply a list of misstakes/errors and silence if that part is fine, an on top of that some kind of editor integration, that puts red markers on said errors
as nice to have would be finding easy to spot errors quickly (flake8 is good at that) but stll adding the more tricky ones later (flake8 cant do that)
sthenault
commented
8 years ago I'm fine with this plan. We should keep in mind though that pylint's exhaustivity make it different from other tools like flake8, and we should probably focus on that.
On Thu, Dec 17, 2015 at 3:40 PM, Claudiu Popa notifications@github.com wrote:
I want to start a discussion regarding what we want to have in pylint 20, from the point of view of the usability of the tool, rather than the static analysis capabilities There are a lot of folks out there who could use pylint, but they don't use it currently because of various reasons, most of them having real arguments that I'd like to address with this issue
When talking about this 20, I'm not referring to the previous 20 that arised in discussions we had over IRC, with advanced static analysis capabilities, such as control flow graphs, points-to analysis and abstract interpretation We'll have to call the latter one 30 though, since it's going to be another major API breaker 20, in my opinion, should have a main focus of reducing the annoyances that users have with the tool from a UX perspective
Without further ado, here are the issues I'd like to address with 20:
1 Sane defaults
Pylint's output is too verbose by default While I agree that most of the messages should be left as is, we could try to activate the -E flag by default It should offer the most useful set of errors that users shouldn't have in their project, leaving style checking and other non-critical checks to be activated explicitly by those interested in them
The reports are not so useful, we could activate "--reports=n" by default They tend to be useful in some situations, but impedes the normal workflow when working with pylint The only capability that I'd like to have activated by default is the score, though, since it tends to make the code quality a gamification for beginners
Remove or downgrade to extensions some checkers, such as too-few-public-methods, too-many-public-methods etc I'll have to compile a full list for these, but I'm interested in what messages don't seem so useful for the community
Reevaluate constants for checker's options, such as the maximum number of statements, maximum number of arguments etc For me, some of them are set too low and I always change them
2 Prefer false negatives over false positives
Currently we're doing the opposite thing, we emit a message even if there is a high risk of a false positive to occur Things changed quite a bit in the last year though regarding this aspect For instance, we don't emit no-member if we don't detect all the base classes of the class which owns the attribute we're trying to access
But we're not there yet, we still have tons of false positives, but this is a topic that's not going to go away so soon, requiring a higher level understanding of Python than we currently have and more static analysis concepts brought into our need (call graphs, abstract interpretation, flow control understanding, points-to and so much more) What we can do is to try not to emit a message if we're not 100% confident that it's actually a problem The first example that comes into my mind is no-member, which we could try not to emit if we can infer multiple values for the same object The idea is to keep away false positives, but letting false negatives taking their position It's much worse to deal with 100 false positives in a project that with 10-15 false negatives (which will eventually be detected correctly, as soon as we'll implement what's missing for them)
So, what do you folks think about this? What else we should target for 20 and what would be of interest to you for using pylint more often? The plan is to have this 20 out in march-april, with the main focus for an advanced 30 later on this year
cc @The-Compiler https://github.com/The-Compiler @dmand https://github.com/dmand @ceridwen https://github.com/ceridwen I'd like your opinion as well on this one
— Reply to this email directly or view it on GitHub https://github.com/PyCQA/pylint/issues/746.
Sylvain
RonnyPfannschmidt
commented
8 years ago an ux and histograms on those statistics would fit a integration in CI tools
the statistics and how they change by what developer are helpfull for team leads, but in my day to day development its just line noise hiding the real errors
JonathanWillitts
commented
8 years ago As a (non-contributing) user who wants things to 'just work' out of the box, a "--reports=n" style output would be much more user-friendly (and much less daunting for new users running for the first time). However, I also agree the score should remain included as it's important not just for gamification, but also as a quick sanity check after making code changes (i.e. have I made anything worse).
Losing the I: and perhaps even W: messages by default would make the output cleaner too.
I think getting Python 3.5 support via the pip installed version is important too for new users who have just installed the latest version of Python (especially now 3.5 has been out for a while).
PCManticore
commented
8 years ago @JonathanWillitts latest pylint version, 1.5.1, has support for Python 3.5 and it's already released on PyPi for a while now.
flub
commented
8 years ago I agree just defaulting to -E --reports=no would help most people's initial reaction to pylint. On the false positives, if checkers which regularly have false-positives are disabled by default then this is much less of an issue.
JonathanWillitts
commented
8 years ago @PCManticore my mistake. I've had the 1.4.4 version bookmarked, which I've been regularly checking for 3.5 support.
The-Compiler
commented
8 years ago I've asked about some opinions of people I know complained about this before in #python.
Personally, I'm on the opposite side of things. I like a linter to be as thorough as possible, even if this means it's noisy or shows a few false-positives. For example, in my .eslintrc (another checker for JS, see the website) I turn on a lot of checks they turned off by default.
I think one of pylint's main features over pyflakes is that it's a lot more powerful and detects a lot more, and I'd like to keep it this way.
I can see how being more quiet/conservative helps for users who'd rather not spend a lot of time configuring pylint, though. However, I'd strongly prefer having some checkers or classes of errors disabled by default, definitely not removed. Removing made sense for rules probably nobody uses (star-magic comes to mind), but I'd discourage it for the current checks.
I definitely agree with the point that --report=n should be the default, minus the score. The report takes up so much space for me so I don't even see the issues anymore.
The rules I disable myself are:
fixmeI have some FIXME's in my code which are there for a long time (so I don't want them to fail my builds), and usually link to some issue, which is why I disable it.
I'm thinking of getting rid of them (as I have the issues already anyways) and re-enabling this though, so I can actually use fixme for work-in-progress stuff I don't want to forget before cleaning up and pushing.
But I think there are a lot of codebases which use it in the way I described above (i.e. as long-standing todo-lists), where this check just is in the way.
global-statementI think there are some occasions where you can't avoid having global state, and when you do it's much better to do so via global than doing some other way (like building a singleton) - at least it makes it clear something is, in fact, global.
I use global 6 times in some 1000 lines of code, but I think they are very much justified there.
locally-disabledsuppressed-messagefile-ignoredThose are just useless meta-noise IMHO, unless one's debugging why some kind of message doesn't show up.
(I'm not sure all of those are enabled by default, I do enable=all)
too-many-ancestorstoo-few-public-methodstoo-many-public-methodstoo-many-instance-attributestoo-many-linesI think it's quite difficult to say if something is too complex. I think there are some useful measures in pylint for that, and I think those definitely aren't useful.
I personally use the other too-few/many-* checkers, but I guess they should be disabled by default too.
cyclic-importI want to look at this in detail so I can't really say much, but right now I don't mind cyclic imports as long as they don't raise ImportError, and I think it's difficult to avoid in big projects. In fact, I think avoiding it probably leads to worse code/separation - but as said, no experience with that yet.
bad-continuationThis gives me a lot of errors where I clearly find my version a lot easier to read, and some of those might be false-positives. I should probably open an issue for those, but for now I think this is much too agressive.
blacklisted-nameI have some stuff called bar (as in a statusbar), and I don't think this is very useful, as this is something I clearly can catch better as a human. There are a lot of bad variable names which can't just be listed on a blacklist.
logging-format-interpolationThis is a special case for me, I think this makes sense generally. I log the debug log to RAM however, so the string will always be interpolated, which means I might as well use .format().
broad-exceptbare-excepteval-usedexec-usedThose might make sense for beginners. For me I always have a good reason when I do this kind of thing, so I want it out of the way.
wrong-import-orderungrouped-importsredefined-variable-typeThose are definitely too strict and buggy in the current release. I might want to look at them later, as I see some fixes for the issues I had already.
I also configure some to be less strict:
[BASIC]
function-rgx=[a-z_][a-z0-9_]{2,50}$
const-rgx=[A-Za-z_][A-Za-z0-9_]{0,30}$
method-rgx=[a-z_][A-Za-z0-9_]{2,50}$
attr-rgx=[a-z_][a-z0-9_]{0,30}$
argument-rgx=[a-z_][a-z0-9_]{0,30}$
variable-rgx=[a-z_][a-z0-9_]{0,30}$
docstring-min-length=3
[FORMAT]
max-line-length=79
ignore-long-lines=<?https?://
expected-line-ending-format=LF
[SIMILARITIES]
min-similarity-lines=8
[VARIABLES]
dummy-variables-rgx=_.*
[DESIGN]
max-args=10@Kwpolska just did run pylint over Nikola and posted the results:
For the (???) ones he doesn't understand what the messages mean exactly - I haven't had time to investigate and look at the code so far.
I think this is a pretty good overview of what's useful, what's not, and maybe a bug (or missing brain module) or two we aren't aware of.
ceridwen
commented
8 years ago I think I'm in agreement with @PCManticore and @Kwpolska. I have some philosophical differences with how pylint currently works: I want a static analysis tool to find bugs, not complain about formatting or code style issues. For formatting, my preferred solution is something like https://github.com/google/yapf or https://github.com/myint/pyformat, which eliminates the busywork of having to fix code formatting at the same time as finding issues with existing formatting.
I think a lot of what I'll call style checks, to distinguish them from formatting or actual bug checks, are worse than useless for me because they create so much noise in pylint's output. I'm used to, when I use pylint on my own larger projects or other people's projects, wading through hundreds of lines of messages looking for the handful that represent possible bugs and not style issues. The too-many/too-few checkers are the worst offenders, but there are others. Theoretically, I could fix this problem by configuring pylint to ignore all the style messages, but this is a lot of upfront work I've never felt like investing. Is disagreeing with users about intentional choices useful for pylint? When I use map or filter, it's because I disagree with the judgment made by pylint that they shouldn't ever be used. When I use eval or exec, it's because there are no alternatives (or the alternatives are worse). When I use classes without methods as data containers, there's a reason for it. In none of these cases will I change my code whether pylint complains about it or not.
There's a good article about the authors' experiences with commercializing Coverity on the sociology of static analysis tools. They discovered that some users would question even indisputable simple bugs. When pylint flags debatable style issues, particularly when they're deliberate, does that convince the users to change their code or that pylint is wasting their time? Pylint is free so the barrier to entry is lower, but it's not zero. Likewise, I agree with @PCManticore that pylint's false positive rate is too high. Meanwhile, the style checkers inflate this by creating more cases where the users are likely to disagree with the tool. I think style checking has its places for companies/projects that have agreed-upon conventions, but are not so useful in general. They should be opt-in, not opt-out.
sthenault
commented
8 years ago I'm personaly on Florian's side on what I expect from pylint. And I think his way of listing messages that he think could/should be removed is probably the most constructive and efficient way to reach a consensus.
Part of this has already been discussed before, and I think we already agree on two things :
I also would like to recall that pylint's philosophy has always been that one should not expect to fix all messages, because some of them are only pointer to things that may be undesired / done differently, while it's of course perfectly fine for the developper to keep it as is. That's also why we have such a sensible control of messages. Also, I'm not really in favor of having a lot of things disabled by default, because I feel like most people won't ever take a look at them. If we go that way I would rather like to see a registry of pylint plugins, not shiped with the core. But that's development, documentation and communication work.
All that is of course not contradictory with the fact that some messages could be considered useless, that default values may be too restrictive, or that the less false positive the better. Below are my preferences about this.
Message that must be removed (I've always seen a consensus on this):
too-many-ancestorstoo-few-public-methodstoo-many-public-methodstoo-many-instance-attributesMessage that could be removed:
too-many-linestoo-many-argumentstoo-many-localsfixmeblacklisted-nameMessages that should be disabled by default
locally-disabledlocally-enabledsuppressed-messagefile-ignoredAlso I'm fine with Claudiu's proposal to have higher values for remaining
too-many-* messages or alike.
A side note to finish: we (logilab) have recently added new messages, some
of which seem indeed problematic yet as Florian noticed
(wrong-import-order, ungrouped-imports, redefined-variable-type). I
personnaly think that those will become nice to have messages once most
problematic issues with them will be sorted out, and so they are worth the
effort.
FYI, you'll find attached a list of all current pylint messages.
On Fri, Dec 18, 2015 at 4:44 AM, ceridwen notifications@github.com wrote:
I think I'm in agreement with @PCManticore https://github.com/PCManticore and @Kwpolska https://github.com/Kwpolska. I have some philosophical differences with how pylint currently works: I want a static analysis tool to find bugs, not complain about formatting or code style issues. For formatting, my preferred solution is something like https://github.com/google/yapf or https://github.com/myint/pyformat, which eliminates the busywork of having to fix code formatting at the same time as finding issues with existing formatting.
I think a lot of what I'll call style checks, to distinguish them from formatting or actual bug checks, are worse than useless for me because they create so much noise in pylint's output. I'm used to, when I use pylint on my own larger projects or other people's projects, wading through hundreds of lines of messages looking for the handful that represent possible bugs and not style issues. The too-many/too-few checkers are the worst offenders, but there are others. Theoretically, I could fix this problem by configuring pylint to ignore all the style messages, but this is a lot of upfront work I've never felt like investing. Is disagreeing with users about intentional choices useful for pylint? When I use map or filter, it's because I disagree with the judgment made by pylint that they shouldn't ever be used. When I use eval or exec, it's because there are no alternatives (or the alternatives are worse). When I use classes without methods as data containers, there's a reason for it. In none of these cases will I change my code whether pylint complains about it or not.
There's a good article about the authors' experiences with commercializing Coverity http://cacm.acm.org/magazines/2010/2/69354-a-few-billion-lines-of-code-later/fulltext on the sociology of static analysis tools. They discovered that some users would question even indisputable simple bugs. When pylint flags debatable style issues, particularly when they're deliberate, does that convince the users to change their code or that pylint is wasting their time? Pylint is free so the barrier to entry is lower, but it's not zero. Likewise, I agree with @PCManticore https://github.com/PCManticore that pylint's false positive rate is too high. Meanwhile, the style checkers inflate this by creating more cases where the users are likely to disagree with the tool. I think style checking has its places for companies/projects that have agreed-upon conventions, but are not so useful in general. They should be opt-in, not opt-out.
— Reply to this email directly or view it on GitHub https://github.com/PyCQA/pylint/issues/746#issuecomment-165661979.
Sylvain
blacklisted-name
invalid-name
missing-docstring
empty-docstring
unneeded-not
singleton-comparison
misplaced-comparison-constant
unidiomatic-typecheck
consider-using-enumerate
bad-classmethod-argument
bad-mcs-method-argument
bad-mcs-classmethod-argument
line-too-long
too-many-lines
trailing-whitespace
missing-final-newline
multiple-statements
superfluous-parens
bad-whitespace
mixed-line-endings
unexpected-line-ending-format
bad-continuation
wrong-spelling-in-comment
wrong-spelling-in-docstring
invalid-characters-in-docstring
multiple-imports
wrong-import-order
ungrouped-imports
wrong-import-position
old-style-class
syntax-error
unrecognized-inline-option
bad-option-value
init-is-generator
return-in-init
function-redefined
not-in-loop
return-outside-function
yield-outside-function
return-arg-in-generator
nonexistent-operator
duplicate-argument-name
abstract-class-instantiated
bad-reversed-sequence
continue-in-finally
method-hidden
access-member-before-definition
no-method-argument
no-self-argument
invalid-slots-object
assigning-non-slot
invalid-slots
inherit-non-class
inconsistent-mro
duplicate-bases
non-iterator-returned
unexpected-special-method-signature
import-error
used-before-assignment
undefined-variable
undefined-all-variable
invalid-all-object
no-name-in-module
unbalanced-tuple-unpacking
unpacking-non-sequence
bad-except-order
raising-bad-type
misplaced-bare-raise
raising-non-exception
notimplemented-raised
catching-non-exception
slots-on-old-class
super-on-old-class
bad-super-call
missing-super-argument
no-member
not-callable
assignment-from-no-return
no-value-for-parameter
too-many-function-args
unexpected-keyword-arg
redundant-keyword-arg
invalid-sequence-index
invalid-slice-index
assignment-from-none
not-context-manager
invalid-unary-operand-type
unsupported-binary-operation
repeated-keyword
not-an-iterable
not-a-mapping
unsupported-membership-test
unsubscriptable-object
logging-unsupported-format
logging-format-truncated
logging-too-many-args
logging-too-few-args
bad-format-character
truncated-format-string
mixed-format-string
format-needs-mapping
missing-format-string-key
too-many-format-args
too-few-format-args
bad-str-strip-call
print-statement
parameter-unpacking
unpacking-in-except
old-raise-syntax
backtick
long-suffix
old-ne-operator
old-octal-literal
import-star-module-level
fatal
astroid-error
parse-error
method-check-failed
raw-checker-failed
bad-inline-option
locally-disabled
locally-enabled
file-ignored
suppressed-message
useless-suppression
deprecated-pragma
too-many-nested-blocks
simplifiable-if-statement
no-self-use
no-classmethod-decorator
no-staticmethod-decorator
redefined-variable-type
cyclic-import
duplicate-code
too-many-ancestors
too-many-instance-attributes
too-few-public-methods
too-many-public-methods
too-many-return-statements
too-many-branches
too-many-arguments
too-many-locals
too-many-statements
too-many-boolean-expressions
unreachable
dangerous-default-value
pointless-statement
pointless-string-statement
expression-not-assigned
unnecessary-pass
unnecessary-lambda
duplicate-key
deprecated-lambda
useless-else-on-loop
exec-used
eval-used
confusing-with-statement
using-constant-test
bad-builtin
lost-exception
assert-on-tuple
attribute-defined-outside-init
bad-staticmethod-argument
protected-access
arguments-differ
signature-differs
abstract-method
super-init-not-called
no-init
non-parent-init-called
unnecessary-semicolon
bad-indentation
mixed-indentation
lowercase-l-suffix
wildcard-import
deprecated-module
relative-import
reimported
import-self
misplaced-future
fixme
invalid-encoded-data
global-variable-undefined
global-variable-not-assigned
global-statement
global-at-module-level
unused-import
unused-variable
unused-argument
unused-wildcard-import
redefined-outer-name
redefined-builtin
redefine-in-handler
undefined-loop-variable
cell-var-from-loop
bare-except
broad-except
duplicate-except
nonstandard-exception
binary-op-exception
property-on-old-class
logging-not-lazy
logging-format-interpolation
bad-format-string-key
unused-format-string-key
bad-format-string
missing-format-argument-key
unused-format-string-argument
format-combined-specification
missing-format-attribute
invalid-format-index
anomalous-backslash-in-string
anomalous-unicode-escape-in-string
bad-open-mode
boolean-datetime
redundant-unittest-assert
deprecated-method
apply-builtin
basestring-builtin
buffer-builtin
cmp-builtin
coerce-builtin
execfile-builtin
file-builtin
long-builtin
raw_input-builtin
reduce-builtin
standarderror-builtin
unicode-builtin
xrange-builtin
coerce-method
delslice-method
getslice-method
setslice-method
no-absolute-import
old-division
dict-iter-method
dict-view-method
next-method-called
metaclass-assignment
indexing-exception
raising-string
reload-builtin
oct-method
hex-method
nonzero-method
cmp-method
input-builtin
round-builtin
intern-builtin
unichr-builtin
map-builtin-not-iterating
zip-builtin-not-iterating
range-builtin-not-iterating
filter-builtin-not-iterating
using-cmp-argument
The-Compiler
commented
8 years ago Also, I'm not really in favor of having a lot of things disabled by default, because I feel like most people won't ever take a look at them.
I fear the same, but I think this can be solved with a good "getting started" documentation.
If we go that way I would rather like to see a registry of pylint plugins, not shiped with the core. But that's development, documentation and communication work.
Some useful plugin system would indeed be a nice thing. Ideally something based on setuptools entry points, so you could simply install pylint-style, pylint-pedantic, etc. etc. via pip :wink:
Message that could be removed
As mentioned I'd prefer to see stuff disabled by default (or moved to plugins) than removed completely. Some of those surely are useful for some poeple.
A side note to finish: we (logilab) have recently added new messages, some of which seem indeed problematic yet as Florian noticed (
wrong-import-order,ungrouped-imports,redefined-variable-type). I personnaly think that those will become nice to have messages once most problematic issues with them will be sorted out, and so they are worth the effort.
I agree with that - I can see their value if they work properly.
RonnyPfannschmidt
commented
8 years ago what might be a nice path to slowly adopting more rules/configuration could be to notify users of the next tier of messages once they cleared the basic issues
so one gets bugged a bit about enabling more checks over time without getting pressed into it y failing builds
flub
commented
8 years ago A lot of these things are rather subjective and I think the trick is to make them opt-in not remove them as I've seen argued here for some. I use pylint in a team of mixed experienced and I do find it helpful that pylint will complain that there are too many statements, that a line is too long, that the import order is wrong (love this new one!) etc etc. These are described in some posts above as bullshit or must be removed but I often find that the overall design of the code improves when you try and re-architect the problem avoiding those. The great thing is all of these can be configured, the number of statements, the number of lines, how long lines are etc etc.
So I'll reiterate, the trick is to make these more subjective checks that will require creating your own pylintrc to configure them for your project's preferences opt-in, not to remove them.
nbastin
commented
8 years ago I definitely want a tool that can evaluate and error on a style, but pylint needs to be far more configurable to be that tool (PEP8 is definitely not the style I want to enforce). You can write extensions to do this now, but it's not the smoothest workflow, plus it can get inefficient if you're duplicating visits a lot (I don't have a great idea how to fix this off the top of my head, but it needs some kind of pre-processor probably).
What bothers me the most is checks that are added that don't seem to be designed at all - unneeded-not is a classic example of something that if it had been thought through and designed from the start with a truly complete set of tests it never would have had the problem of reporting on __ne__ implementations. redefined-variable-type also seems like something that bit off far more than it could chew. Any time a check is going to be added it should be actually designed on paper and reviewed before it gets implemented, or at least before it gets added to the default set of checkers.
The-Compiler
commented
8 years ago @nbastin Do you have an example of something you'd like to configure but can't currently?
As for unneeded-not and redefined-variable-type, I think by now everyone agrees it was a mistake to merge them quickly before the release. I disagree designing stuff on paper would've changed anything about that, though.
nbastin
commented
8 years ago @The-Compiler I haven't had time to put together a complete list (by far), but a few examples would be:
We want a whitespace between the name and open-parnes in a function/method definition, but not in its' use:
def method (self, some_int):
...vs.
method(7)Also more configurable parameters for things like too-many-arguments, to allow for a distinction between the count of non-default arguments versus those with default values.
I tried to dig up examples of user-configurable stuff from the C++ tool we use, but the license agreement won't let me share them.. :-/
PCManticore
commented
8 years ago Thank you all for your responses, they are real insightful for how you're using pylint and what you're expecting from it.
I'll try to summarize the next course of action in this comment.
pylint --confidence=full should trigger the current behaviour.On the next item, the opinions seems to be split. Some of you are using the tool without expecting upfront for some messages to be disabled by default, other would like for categories such as conventions to be disabled by default. I'm not sure where to fall here, but here's an idea that's a bit inspired by Ronny's idea of tier messages.
We could split the possible categories into four major classes: core pylint messages (violation of the type system, accessing a member which doesn't exist etc), pedantic (the current warnings. They can be actual error or things that we can't figure out), refactoring (things that could be refactored in order to be easier to understand), style (style checkers, that are not necessarily vital in a project)
What I want to do with these coarse categories is to provide tier levels, that needs to be solved first by the user before going to the next one. By default the core is enabled.
$ pylint myproject
# core checkers enabled
10/10 - Congrats, you're clean on a core. You might try with "--pedantic" now.
$ pylint myproject --pedantic
# pedantic checkers enabled.
10/10 - Congrats, you're clean on pedantic. You might try with --refactoring now
$ pylint myproject --refactoring
# refactoring checkers enabled
10/10 - Congrats, you're clean on refactoring. Last up, try with --style now.
$ pylint myproject --style
10/10 - Now you're pylint clean.Now the project is pylint clean. At each step, the user can decide to ignore some messages per each stage, without encountering them anymore in the next runs.
What do you think about this last step? Apart of this, I think we can already start doing the rest of the points we mentioned.
Kwpolska
commented
8 years ago There should also be a way to run all four suites at once:
$ pylint myproject --everything
Core: 10/10
Pedantic: 10/10
Refactoring: 10/10
Style: 10/10
AGGREGATE: 10/10 -- congrats, you're pylint clean.Hi Claudiu,
Seems like a great approach.
One minor thing which you might want to consider is to use something like --level=core|pedantic|refactoring|style instead of the different options to make it more obvious they are layered on top of each other.
Regards, Floris
On 24 December 2015 at 17:47, Claudiu Popa notifications@github.com wrote:
Thank you all for your responses, they are real insightful for how you're using pylint and what you're expecting from it.
I'll try to summarize the next course of action in this comment.
- disable reports by default, enable the score by default
- disable the information category by default
- increase option values for the some checkers (too many arguments etc.)
- regarding false positives, abort a check when multiple values with different types are inferred. Add a --confidence flag, which will control this behaviour. By default the --confidence should have a value for preferring false negatives, but a simple pylint --confidence=full should trigger the current behaviour.
On the next item, the opinions seems to be split. Some of you are using the tool without expecting upfront for some messages to be disabled by default, other would like for categories such as conventions to be disabled by default. I'm not sure where to fall here, but here's an idea that's a bit inspired by Ronny's idea of tier messages.
We could split the possible categories into four major classes: core pylint messages (violation of the type system, accessing a member which doesn't exist etc), pedantic (the current warnings. They can be actual error or things that we can't figure out), refactoring (things that could be refactored in order to be easier to understand), style (style checkers, that are not necessarily vital in a project)
What I want to do with these coarse categories is to provide tier levels, that needs to be solved first by the user before going to the next one. By default the core is enabled.
$ pylint myproject
core checkers enabled
10/10 - Congrats, you're clean on a core. You might try with "--pedantic" now.
$ pylint myproject --pedantic
pedantic checkers enabled.
10/10 - Congrats, you're clean on pedantic. You might try with --refactoring now
$ pylint myproject --refactoring
refactoring checkers enabled
10/10 - Congrats, you're clean on refactoring. Last up, try with --style now.
$ pylint myproject --style 10/10 - Now you're pylint clean.
Now the project is pylint clean. At each step, the user can decide to ignore some messages per each stage, without encountering them anymore in the next runs.
What do you think about this last step? Apart of this, I think we can already start doing the rest of the points we mentioned.
— Reply to this email directly or view it on GitHub https://github.com/PyCQA/pylint/issues/746#issuecomment-167135964.
The-Compiler
commented
8 years ago I like that approach as well, and I agree with what @flub said.
rowillia
commented
7 years ago @PCManticore Reviving this thread a bit :D Has there been much traction on the new tiering system?
As we do this, we should also re-evaluate the categories each error is in. https://medium.com/@acboulder/type-hints-are-scary-f52d07a36a31#.gotbe56pc mentioned some things that were legitimate errors (like mixing tabs and whitespace) that get marked as warnings.
In terms of whether or not PyLint should have opinions about style, totally agree that people should opt into it, but that said I am an ardent defender of being a style nit 😄 . Consistent style has been shown to improve comprehension of code bases and reduce bugs - http://synesthesiam.com/posts/what-makes-code-hard-to-understand.html . This is why I love tools like gofmt and clangfmt. Completely off topic from this, one of the thing that could be useful in pylint is building a system for suggested fixes like ErrorProne - https://github.com/google/error-prone/blob/master/core/src/main/java/com/google/errorprone/bugpatterns/ArrayEquals.java#L93-L96. It seems like Astroid could be used for this, but I haven't dug into it enough.
A former colleague of mine @peterogithub gave a great talk recently on integrating static analysis with projects in CI - https://www.youtube.com/watch?v=lcVx3g3SmmY . The most relevant bits for folks here is the focus on Fix Rate as opposed to false positive.
PCManticore
commented
7 years ago Thank you for bringing this up, @rowillia. Unfortunately this year was slow for me with regard to pylint's development, so, no, there hasn't been a lot of traction for this system. It is still scheduled for 2.0, with the intention of being released soon, since it will be the version with Python 3.6 support. The tiering system shouldn't be a complicated feature, so I have faith that we can deliver it with the next release.
Thank you for that talk, really enjoying it.
rogalski
commented
7 years ago @PCManticore Is any work in progress regarding tier system? If not, I'd like to take a shot at it.
ghost
commented
7 years ago I would recommend to think carefully about all those extra flags and outputs - I think more and more users are taking advantage of linters through editors like Atom or VSCode. It is much more convenient as it highlights problems in the code.
If you start to add custom output messages and opt-in flags nobody would implement them into the plugins. Especially so for wrappers that include pylint (such as very popular pylama).
Most plugins do include options to suppress a particular error - so if changes are too be made, please consider adding an option through it to get back to original 'pedantic' pylint behavior. I am a big fan of style linters.
Just my 0.02$. Keep up the great work!
pradyunsg
commented
5 years ago @PCManticore You might want to remove "in 2.0" from the issue title.
gilmrjc
commented
5 years ago I think the same as here:
Personally, I'm on the opposite side of things. I like a linter to be as thorough as possible, even if this means it's noisy or shows a few false-positives. For example, in my .eslintrc (another checker for JS, see the website) I turn on a lot of checks they turned off by default.
I think one of pylint's main features over pyflakes is that it's a lot more powerful and detects a lot more, and I'd like to keep it this way.
I can see how being more quiet/conservative helps for users who'd rather not spend a lot of time configuring pylint, though. However, I'd strongly prefer having some checkers or classes of errors disabled by default, definitely not removed. Removing made sense for rules probably nobody uses (
star-magiccomes to mind), but I'd discourage it for the current checks.
Going forward with the example, why not doing what eslint does? To improve the configuration experience from users, eslint provides the ability to use "presets". These are packages that active the rules according to the preset. The most common are eslint-airbnb and eslint-standard. You can even combine them to establish the project's rules.
PyLint already provides a way to implement this functionality thanks to BaseChecker.config. A reasonable solution would be to create a PyLint preset package with multiple leves like "pylint_presets.core", "pylint_presets.base", "pylint_presets.strongly-recommended", "pylint_presets.recommended", etc.
With these options a user can enable the core preset showing only errors, or the base preset showing sane basic defaults, or the recommended with a set of rules to enforce best practices. And even the user can use a custom preset with their rules of preference.
This system could work with an autodiscovery system like flake8 does with plugins, and the users add what they feel is better for them without worrying for the configuration, keeping the ability to override the rules they want if the preset isn't exactly what they need.
AWhetter
commented
4 years ago This is an old issue, but there's a lot of important points. So I'm going to dump some more thoughts:
"pylint is opinionated":
I've heard this a lot and never got a clear answer on what it means. If anyone has this opinion, please share why you think this is.
I've interpreted it as "pylint isn't configurable". I've tried to take this to heart when adding new checks or editing existing ones, but we have a way to go with the existing ones. For example, too-few-public-methods is valid in some cases and theoretically you could configure when it's fine rather than needing to either silence the message with pragmas or turn the message off completely.
I think which messages we have turned on by default plays a role in this. It's been discussed already, to the point where I think we can action on it. Having an audit of which checks are currently enabled and which should be seems like a good next step to me?
False positives:
I totally agree with the sentiments in the issue description. We get issues in all the time about no-member messages on C modules, for example. I'm not sure what the solution is for it yet, but it's definitely an issue.
Configuration:
The config system is not my favourite part of pylint. The generated pylintrc files have so much stuff in them and it's daunting. It's better now that we allow reading from pyproject.toml files, but it always bothered me that the options are separated into different sections. It makes it more daunting to get started with changing the configuration file. The sections seem arbitrary.
Pragmas are also a bit ugly. To the point where we suggested combining with #noqa in (#2493). Even though we can't combine with #noqa, I think we can still improve on the length and simplicity of # pylint: disable=line-too-long.
The code for configuration is challenging to work with as well. To the point where it prevented the per_dir_config project (#618) from being even close to easy. We've spoken before about what would a theoretical pylint rewrite look like. I think about it often because I think it would be a good thing. We struggle to find maintainers, and I think it's partly because the codebase is so difficult to get started with. But I don't think we would need to go from scratch with a rewrite. I would keep the contents of the checkers and their logic the same, but everything around that like how to launch the checkers, reading config, reporting, loading plugins, etc I would rewrite. This would let us fix things like the brokenness of parallel checking as well. Something this drastic is obviously only possible with a major version bump. Maybe it's a pipe dream. We struggle with development as it is. But hey, a guy can dream.
Plugins: I agree with what's been said. I would even split things like the score out into a plugin. Reports could be a plugin. We have a huge burden as maintainers and it's partly because we support something so big. Splitting it up into plugins might allow for more external development outside the core project. It's the direction that flake8 has and it works well.
AWhetter
commented
4 years ago Style: The more time goes on, the more I think we could remove style checking altogether. I've always preferred flake8 for style checking. It's a bit more configurable and has a wider range of checks. But now that things like black exist, they generally do a better job of styling than we do. Plus the code is a nightmare! Perhaps it's another set of plugins we can think about splitting out.
The-Compiler
commented
4 years ago I've heard this a lot and never got a clear answer on what it means. If anyone has this opinion, please share why you think this is.
I might repeat a bit of what I said in https://github.com/PyCQA/pylint/issues/746#issuecomment-165484010 already, but it's been a while!
I guess it depends on how you approach using a linter. My personal approach is "give me the firehose of everything you can possibly find, and I'm going to spend the time turning off everything I don't agree with". As such, I've been very happy with pylint, as it's much more powerful than flake8 if you spend the time needed to configure it.
However, most people I've spoken to consider the default configuration way too noisy - they'd rather have a linter which they can start using right away, without any configuration needed, and get sensible results out-of-the-box.
So, personally, I'd still turn on everything and filter out rules individually - but many others would probably like something a bit more quiet (but still useful).
See e.g. how eslint has a list of rules and only enables "rules that report common problems". They also only change that set with a major release, so people can safely upgrade the linter without having new "problems" to take care of.
However, playing devil's advocate, I'm wondering how useful pylint really is at that point compared to flake8/black (for style things) plus mypy (with --check-untyped-defs). I've been thinking about dropping pylint entirely for my projects (since it's much slower than mypy/flake8/black), but I'm not sure how much benefit it still brings me over other tools.
AWhetter
commented
4 years ago I was at PyCascades last weekend and got a few more bits of feedback for us to think on.
Default messages: I think we already agree on this so I won't say much, but there was agreement that changing the default messages would be a good thing. Some people turn everything off when they start using pylint and build up a list of enabled messages themselves.
Working with flake8: pylint is used so frequently with flake8 that it would be useful to have documentation on how to use both together. So noting down things like what messages are duplicated between both, and any messages that pylint and flake8 have disagreements on. There's probably something to be said about style checks as well.
Style: Continuing from the last point, flake8 definitely seemed to be the preferred method of style checking over pylint. People said they wouldn't miss the style checks if we do decide to either remove them or move them to a plugin, because many were using flake8 for that.
Ease of use: There was a comment that junior developers struggle to get started with pylint, and part of that is because the messages generally only tell you what's wrong and not how to fix it. (Where I was last we had integration tests on our linters. Tests would check for lint errors and have an equivalent passing test. We then used these in the documentation for error code to link to examples. All of this was generated automatically. We could do something similar.) The emphasis on using message names rather than message codes is a big plus over flake8.
DanielNoord
commented
2 years ago To get this thread some traction again I think we need to redefine what needs to be done to close this. To recap what I believe are the main points and what still needs to be done.
Focusing on that last issue:
I totally agree with what the @ghost said and don't think we should add too many options to the command-line again. Every option we add also gets added to the default generated pylintrc, which, as has been noted in this topic before, is already very large and daunting.
A much better option would be the presets discussed by @gilmrjc. I have also used those before on eslint and they work really well. It also allows organisations and individuals to quickly share their preset of messages with others.
I'd like the opinion of others on this as well but for now I would propose to change this issue name to "Allow configuring message tiers or presets" and see what the consensus on this last issue will be.
Pierre-Sassoulas
commented
2 years ago I think we also have:
Pierre-Sassoulas
commented
2 years ago Closing in favor of #7120 and #7121.
I want to start a discussion regarding what we want to have in pylint 2.0, from the point of view of the usability of the tool, rather than the static analysis capabilities. There are a lot of folks out there who could use pylint, but they don't use it currently because of various reasons, most of them having real arguments that I'd like to address with this issue.
When talking about this 2.0, I'm not referring to the previous 2.0 that arised in discussions we had over IRC, with advanced static analysis capabilities, such as control flow graphs, points-to analysis and abstract interpretation. We'll have to call the latter one 3.0 though, since it's going to be another major API breaker. 2.0, in my opinion, should have a main focus of reducing the annoyances that users have with the tool from a UX perspective.
Without further ado, here are the issues I'd like to address with 2.0:
Pylint's output is too verbose by default. While I agree that most of the messages should be left as is, we could try to activate the -E flag by default. It should offer the most useful set of errors that users shouldn't have in their project, leaving style checking and other non-critical checks to be activated explicitly by those interested in them.
The reports are not so useful, we could activate "--reports=n" by default. They tend to be useful in some situations, but impedes the normal workflow when working with pylint. The only capability that I'd like to have activated by default is the score, though, since it tends to make the code quality a gamification for beginners.
Remove or downgrade to extensions some checkers, such as too-few-public-methods, too-many-public-methods etc. I'll have to compile a full list for these, but I'm interested in what messages don't seem so useful for the community.
Reevaluate constants for checker's options, such as the maximum number of statements, maximum number of arguments etc. For me, some of them are set too low and I always change them.
Currently we're doing the opposite thing, we emit a message even if there is a high risk of a false positive to occur. Things changed quite a bit in the last year though regarding this aspect. For instance, we don't emit no-member if we don't detect all the base classes of the class which owns the attribute we're trying to access.
But we're not there yet, we still have tons of false positives, but this is a topic that's not going to go away so soon, requiring a higher level understanding of Python than we currently have and more static analysis concepts brought into our need (call graphs, abstract interpretation, flow control understanding, points-to and so much more). What we can do is to try not to emit a message if we're not 100% confident that it's actually a problem. The first example that comes into my mind is no-member, which we could try not to emit if we can infer multiple values for the same object. The idea is to keep away false positives, but letting false negatives taking their position. It's much worse to deal with 100 false positives in a project that with 10-15 false negatives (which will eventually be detected correctly, as soon as we'll implement what's missing for them).
So, what do you folks think about this? What else we should target for 2.0 and what would be of interest to you for using pylint more often? The plan is to have this 2.0 out in march-april, with the main focus for an advanced 3.0 later on this year.
cc @The-Compiler @dmand @ceridwen I'd like your opinion as well on this one.