introduce strict-ish pylint + minimal fixes #98
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Generated via `pylint --generate-rcfile`
Consistent with `black` formatter.
PyLint 2.16 has started complaining: > pylint: Command line or configuration file:1: UserWarning: Specifying > exception names in the overgeneral-exceptions option without module > name is deprecated and support for it will be removed in pylint 3.0. > Use fully qualified name (maybe 'builtins.BaseException' ?) instead.
The checks will be selectively removed from "disabled=…" as fixed.
The type declaration of Generic.__new__ is over-broad in Python 3.8 (likely also in 3.9, though not verified), but fixed in later versions. This causes warnings if `Generic` subclasses override `__new__` but don't expose an "anything goes" (*args, **kwargs) signature. It's not worth writing a specialized transformer to fix it, and it's lame to have to write lint suppressions on correct code, especially when it only applies to a particular version.
pylint: unnecessary-pass
Suppressing two existing TODOs in rxelems.test pylint: fixme
Suppressed in intentional `__eq__` tests. pylint: comparison-with-itself
Suppressing on Fsm class. pylint: too-many-public-methods
Suppressing on existing large functions. pylint: too-many-branches pylint: too-many-locals pylint: too-many-return-statements pylint: too-many-statements
Suppressed in tests where it's actually checking string output. > Avoid comparisons to empty string pylint: compare-to-empty-string
pylint: unused-variable
pylint: expression-not-assigned pylint: pointless-statement
> Access to a protected member _commonconc of a client class pylint: protected-access
> test for membership should be 'not in' flake8: E713
pylint: unnecessary-comprehension
> consider using [] instead of list() pylint: use-list-literal
>> 'list(...) != []' can be simplified to 'list(...)' as an empty >> sequence is falsey pylint: use-implicit-booleaness-not-comparison
> Avoid comparisons to zero pylint: compare-to-zero
pylint: redefined-builtin
To avoid altering signatures, this does not include keyword arguments or attributes of `Fsm` here, just local variable names. > Redefining built-in 'map' pylint: redefined-builtin
pylint: redefined-builtin
pylint: consider-iterating-dictionary
Note that there are many other instances of unnecessary lookups; this is the only one that triggers this checker. pylint: unnecessary-dict-index-lookup
pylint: consider-using-dict-items
> Consider using enumerate instead of iterating with range and len pylint: consider-using-enumerate
> Consider using a dictionary comprehension pylint: consider-using-dict-comprehension
> Used builtin function 'filter'. Using a list comprehension can be > clearer. pylint: bad-builtin
> Redefinition of initial type from set to frozenset > Redefinition of mults type from list to reversed pylint: redefined-variable-type
Trying to keep it low-ish impact. pylint: invalid-name
…arset The collection already contains `str` elements.
> Formatting a regular string which could be a f-string pylint: consider-using-f-string
> Consider rewriting as a ternary expression pylint: consider-ternary-expression
> Redefining 'symbol' from loop pylint: redefined-loop-name
> Implementing __eq__ without also implementing __hash__ pylint: eq-without-hash
> `for` loop could be `any(…)` pylint: consider-using-any-or-all
> try clause contains 9 statements, expected at most 1 pylint: too-many-try-statements
> Raising too general exception: Exception pylint: broad-exception-raised
And enables the `useless-suppression` checker, which is disabled by default. Removes some statements which would not trigger warnings, for some reason or other: generally these are left over from refactors. For example, the too-many-arguments on `Fsm.__init__` had been triggering before the arguments were explicitly marked as keyword-only, but the suppression was not removed with that change. pylint: useless-suppression
qntm
reviewed
Aug 3, 2023
qntm
approved these changes
Aug 3, 2023
I think it's just trying to suggest consistency about something that's sometimes beneficial and basically never harmful. The general recommendation is to use the least-powerful test. An expression like In most cases, this doesn't matter, since built-in collection types maintain their length eagerly, and As a pattern, (Historically, in Python 2, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a default
pylintrc, updates some project-specific fields, enables most of the built-in plugins, and suppresses all of the currently-triggering checkers.Following that, some low-hanging fruit is addressed, suppressing pretty liberally, with care to limit logical impact and interface changes. (Exception: renaming
state_type/alpha_typetoStateType/AlphaType. They could be given as aliases, but seemed unlikely for anyone to care).The most visible changes are:
ValueErrororArithmeticErrorexceptions vs.Exception.Exceptionincludes things likeSyntaxErrorandTypeErrorand is just way too broad.next()andmin()/max()not xvslen(x) == 0(generally good practice, since some collections can determine non-empty faster than they can count the entire collection) or using.items()orenumerate(…)to avoid repeated lookups in indexed collections.This isn't motivated by much, just a branch I had following me around.