From 1125296b53303b1495c2ae93d5814d4b1cd9d293 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sun, 25 Feb 2024 23:56:00 +0100 Subject: [PATCH] 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)