Skip to content

Conversation

maurycy
Copy link
Contributor

@maurycy maurycy commented Sep 1, 2025

After #404, I ran uvx ruff check and found a lot of not so subtle errors:

12:51:00.954839000AM CEST maurycy@gimel /Users/maurycy/src/pyperformance % uvx ruff check --output-format concise
pyperformance/_benchmark.py:221:67: F821 Undefined name `NOOP`
pyperformance/_benchmark_metadata.py:197:13: F841 Local variable `repo` is assigned to but never used
pyperformance/_manifest.py:346:17: F821 Undefined name `seclines`
pyperformance/_manifest.py:366:9: F841 Local variable `yielded` is assigned to but never used
pyperformance/_pyproject_toml.py:103:15: F821 Undefined name `ValuError`
pyperformance/_pyproject_toml.py:125:30: F541 [*] f-string without any placeholders
pyperformance/_pyproject_toml.py:129:9: F841 Local variable `text` is assigned to but never used
pyperformance/_utils.py:9:5: F822 Undefined name `run_command` in `__all__`
pyperformance/run.py:205:25: F821 Undefined name `os`
pyperformance/tests/test_commands.py:105:13: F841 Local variable `text` is assigned to but never used
pyperformance/tests/test_commands.py:111:13: F841 Local variable `text` is assigned to but never used
pyperformance/tests/test_commands.py:151:9: F841 Local variable `text` is assigned to but never used
Found 12 errors.
[*] 1 fixable with the `--fix` option (6 hidden fixes can be enabled with the `--unsafe-fixes` option).

Including benchmarks:

12:33:31.816619000AM CEST maurycy@gimel /Users/maurycy/src/pyperformance % uvx ruff check --output-format concise
pyperformance/_benchmark.py:4:5: F822 Undefined name `Benchmarkcheck_name` in `__all__`
pyperformance/_benchmark.py:221:67: F821 Undefined name `NOOP`
pyperformance/_benchmark_metadata.py:197:13: F841 Local variable `repo` is assigned to but never used
pyperformance/_manifest.py:346:17: F821 Undefined name `seclines`
pyperformance/_manifest.py:366:9: F841 Local variable `yielded` is assigned to but never used
pyperformance/_pyproject_toml.py:103:15: F821 Undefined name `ValuError`
pyperformance/_pyproject_toml.py:125:30: F541 [*] f-string without any placeholders
pyperformance/_pyproject_toml.py:129:9: F841 Local variable `text` is assigned to but never used
pyperformance/_utils.py:9:5: F822 Undefined name `run_command` in `__all__`
pyperformance/data-files/benchmarks/bm_2to3/run_benchmark.py:20:16: F401 `lib2to3` imported but unused; consider using `importlib.util.find_spec` to test for availability
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/btm_matcher.py:162:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/btm_utils.py:170:17: F841 Local variable `leaf` is assigned to but never used
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/fixes/fix_except.py:48:9: F841 Local variable `syms` is assigned to but never used
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/fixes/fix_exec.py:28:9: F841 Local variable `syms` is assigned to but never used
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/fixes/fix_exitfunc.py:66:13: F841 Local variable `stmt_container` is assigned to but never used
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/fixes/fix_has_key.py:80:9: F841 Local variable `anchor` is assigned to but never used
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/fixes/fix_itertools_imports.py:22:17: F841 Local variable `member` is assigned to but never used
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/fixes/fix_urllib.py:49:5: F841 Local variable `bare` is assigned to but never used
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/fixes/fix_urllib.py:129:21: F841 Local variable `as_name` is assigned to but never used
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/pgen2/pgen.py:160:13: F841 Local variable `oldlen` is assigned to but never used
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/pgen2/pgen.py:162:13: F841 Local variable `newlen` is assigned to but never used
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/pgen2/tokenize.py:32:1: E401 [*] Multiple imports on one line
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/pgen2/tokenize.py:34:1: F403 `from lib2to3.pgen2.token import *` used; unable to detect undefined names
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/pgen2/tokenize.py:492:25: F841 Local variable `strstart` is assigned to but never used
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/pygram.py:10:20: F401 [*] `.pgen2.token` imported but unused
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/pygram.py:12:15: F401 [*] `.pytree` imported but unused
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/pytree.py:28:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
pyperformance/data-files/benchmarks/bm_2to3/vendor/src/lib2to3/refactor.py:460:44: E713 [*] Test for membership should be `not in`
pyperformance/data-files/benchmarks/bm_barnes_hut/run_benchmark.py:342:5: F841 Local variable `initial_energy` is assigned to but never used
pyperformance/data-files/benchmarks/bm_barnes_hut/run_benchmark.py:353:9: F841 Local variable `final_energy` is assigned to but never used
pyperformance/data-files/benchmarks/bm_dask/run_benchmark.py:8:18: F401 [*] `dask.distributed` imported but unused
pyperformance/data-files/benchmarks/bm_gc_traversal/run_benchmark.py:21:5: F841 Local variable `all_cycles` is assigned to but never used
pyperformance/data-files/benchmarks/bm_sphinx/data/Doc/conf.py:15:13: F541 [*] f-string without any placeholders
pyperformance/run.py:205:25: F821 Undefined name `os`
pyperformance/tests/test_commands.py:105:13: F841 Local variable `text` is assigned to but never used
pyperformance/tests/test_commands.py:111:13: F841 Local variable `text` is assigned to but never used
pyperformance/tests/test_commands.py:151:9: F841 Local variable `text` is assigned to but never used
Found 37 errors.
[*] 7 fixable with the `--fix` option (20 hidden fixes can be enabled with the `--unsafe-fixes` option).

It makes one wonder how much of this code is actually dead...

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@maurycy maurycy requested a review from AA-Turner September 1, 2025 23:06
maurycy and others added 6 commits September 2, 2025 13:25
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@maurycy
Copy link
Contributor Author

maurycy commented Sep 2, 2025

@hugovk Voilà!

Thank you for a beautiful review.

Some notes:

@hugovk
Copy link
Member

hugovk commented Sep 2, 2025

  • The benchmarks do not look that bad (mostly unused variables) but maybe we should enforce some stardards there as well?

It's very important we can compare new benchmark runs with old ones, so let's just leave them be. We want to benchmark some sort of "real-life code", and unused variables is often part of that :)

  • What do you think think about reducing ignore over time? #405 (files)

Yes, makes sense. We can do it in followups. We might want to autoformat the code and keep ignoring E501.

  • The whole affair proves there's some dead code. What do you think is the best strategy to reduce this? (I've had mixed experiences with vulture.)

Yeah, again it's safer to just leave the actual benchmarks as-is, but we could do a sweep of the benchmark-running code. I've used vulture a bit, there's also deadcode which is similar, but deadcode has nice colour output.

maurycy and others added 4 commits September 2, 2025 15:02
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, one last thing :)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@hugovk hugovk merged commit f3c4495 into python:main Sep 3, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants