From bd9b62161a1caa152ceeaf0cec8586978b5d7e21 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 30 Mar 2024 20:55:27 +0100 Subject: [PATCH 1/2] [tooling] Add a manual step to run pylint in pre-commit --- .pre-commit-config.yaml | 7 +++ pyproject.toml | 114 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1f0214b7e..e7f456791 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -50,6 +50,13 @@ repos: additional_dependencies: ["tox>=4.9"] - repo: local hooks: + - id: pylint + name: pylint + entry: pylint + language: system + types: [python] + args: ["-rn", "-sn", "--fail-on=I"] + stages: [manual] - id: rst name: rst entry: rst-lint --encoding utf-8 diff --git a/pyproject.toml b/pyproject.toml index 7d1b8a22d..b33f449b2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -165,6 +165,120 @@ lines-after-imports = 2 "src/_pytest/_version.py" = ["I001"] "testing/python/approx.py" = ["B015"] +[tool.pylint.main] +# Maximum number of characters on a single line. +max-line-length = 120 +disable= [ + "abstract-method", + "arguments-differ", + "arguments-renamed", + "assigning-non-slot", + "attribute-defined-outside-init", + "bad-classmethod-argument", + "bad-mcs-method-argument", + "broad-exception-caught", + "broad-exception-raised", + "cell-var-from-loop", + "comparison-of-constants", + "comparison-with-callable", + "comparison-with-itself", + "condition-evals-to-constant", + "consider-iterating-dictionary", + "consider-using-dict-items", + "consider-using-enumerate", + "consider-using-from-import", + "consider-using-f-string", + "consider-using-in", + "consider-using-sys-exit", + "consider-using-ternary", + "consider-using-with", + "cyclic-import", + "disallowed-name", + "duplicate-code", + "eval-used", + "exec-used", + "expression-not-assigned", + "fixme", + "global-statement", + "implicit-str-concat", + "import-error", + "import-outside-toplevel", + "inconsistent-return-statements", + "invalid-bool-returned", + "invalid-name", + "invalid-repr-returned", + "invalid-str-returned", + "keyword-arg-before-vararg", + "line-too-long", + "method-hidden", + "misplaced-bare-raise", + "missing-docstring", + "missing-timeout", + "multiple-statements", + "no-else-break", + "no-else-continue", + "no-else-raise", + "no-else-return", + "no-member", + "no-name-in-module", + "no-self-argument", + "not-an-iterable", + "not-callable", + "pointless-exception-statement", + "pointless-statement", + "pointless-string-statement", + "protected-access", + "raise-missing-from", + "redefined-argument-from-local", + "redefined-builtin", + "redefined-outer-name", + "reimported", + "simplifiable-condition", + "simplifiable-if-expression", + "singleton-comparison", + "superfluous-parens", + "super-init-not-called", + "too-few-public-methods", + "too-many-ancestors", + "too-many-arguments", + "too-many-branches", + "too-many-function-args", + "too-many-instance-attributes", + "too-many-lines", + "too-many-locals", + "too-many-nested-blocks", + "too-many-public-methods", + "too-many-return-statements", + "too-many-statements", + "try-except-raise", + "typevar-name-incorrect-variance", + "unbalanced-tuple-unpacking", + "undefined-loop-variable", + "undefined-variable", + "unexpected-keyword-arg", + "unidiomatic-typecheck", + "unnecessary-comprehension", + "unnecessary-dunder-call", + "unnecessary-lambda", + "unnecessary-lambda-assignment", + "unpacking-non-sequence", + "unspecified-encoding", + "unsubscriptable-object", + "unused-argument", + "unused-import", + "unused-variable", + "used-before-assignment", + "use-dict-literal", + "use-implicit-booleaness-not-comparison", + "use-implicit-booleaness-not-len", + "useless-else-on-loop", + "useless-import-alias", + "useless-return", + "use-maxsplit-arg", + "using-constant-test", + "wrong-import-order", +] + [tool.check-wheel-contents] # check-wheel-contents is executed by the build-and-inspect-python-package action. # W009: Wheel contains multiple toplevel library entries From 1125296b53303b1495c2ae93d5814d4b1cd9d293 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sun, 25 Feb 2024 23:56:00 +0100 Subject: [PATCH 2/2] Small performance/readability improvments when iterating dictionnary with ``keys()`` Based on pylint's message ``consider-iterating-dictionary`` suggestion. Surprisingly using a dict or set comprehension instead of a new temp var is actually consistently slower here, which was not intuitive for me. ```python from timeit import timeit families = {1: {"testcase": [1, 2, 3, 5, 8]}} attrs = {1: "a", 2: "b", 3: "c", 4: "d", 5: "e", 6: "f", 7: "g", 8: "h"} class Old: def old(self): self.attrs = attrs temp_attrs = {} for key in self.attrs.keys(): if key in families[1]["testcase"]: temp_attrs[key] = self.attrs[key] self.attrs = temp_attrs class OldBis: def old(self): self.attrs = attrs temp_attrs = {} for key in self.attrs: if key in families[1]["testcase"]: temp_attrs[key] = self.attrs[key] self.attrs = temp_attrs class New: def new(self): self.attrs = attrs self.attrs = { # Even worse with k: v for k in self.attrs.items() k: self.attrs[k] for k in self.attrs if k in families[1]["testcase"] } if __name__ == "__main__": n = 1000000 print(f"Old: {timeit(Old().old, number=n)}") print(f"Just removing the keys(): {timeit(OldBis().old, number=n)}") print(f"List comp, no temp var: {timeit(New().new, number=n)}") ``` Result: Old: 0.9493889989680611 Just removing the keys(): 0.9042672360083088 List comp, no temp var: 0.9916125109884888 It's also true for the other example with similar benchmark, but the exact code probably does not need to be in the commit message. --- pyproject.toml | 1 - src/_pytest/junitxml.py | 2 +- src/_pytest/terminal.py | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b33f449b2..4d4b52287 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -183,7 +183,6 @@ disable= [ "comparison-with-callable", "comparison-with-itself", "condition-evals-to-constant", - "consider-iterating-dictionary", "consider-using-dict-items", "consider-using-enumerate", "consider-using-from-import", diff --git a/src/_pytest/junitxml.py b/src/_pytest/junitxml.py index e6ccebc20..13fc9277a 100644 --- a/src/_pytest/junitxml.py +++ b/src/_pytest/junitxml.py @@ -143,7 +143,7 @@ class _NodeReporter: # Filter out attributes not permitted by this test family. # Including custom attributes because they are not valid here. temp_attrs = {} - for key in self.attrs.keys(): + for key in self.attrs: if key in families[self.family]["testcase"]: temp_attrs[key] = self.attrs[key] self.attrs = temp_attrs diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 2c9c0d3b1..973168dc6 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -1267,7 +1267,7 @@ class TerminalReporter: def _set_main_color(self) -> None: unknown_types: List[str] = [] - for found_type in self.stats.keys(): + for found_type in self.stats: if found_type: # setup/teardown reports have an empty key, ignore them if found_type not in KNOWN_TYPES and found_type not in unknown_types: unknown_types.append(found_type)