Skip to content

Add C4 linting rules - #7543

Open
agoscinski wants to merge 2 commits into
aiidateam:mainfrom
agoscinski:pyupgrade-precommit
Open

Add C4 linting rules#7543
agoscinski wants to merge 2 commits into
aiidateam:mainfrom
agoscinski:pyupgrade-precommit

Conversation

@agoscinski

Copy link
Copy Markdown
Collaborator

I am not sure why these things have not been upgraded yet in the last time we ran pyupgrade but it made me conclude that we should just add it in hook since part of it is just formatting style choice that can be broken in future commits.

@agoscinski
agoscinski requested a review from GeigerJ2 August 12, 2026 20:00
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: 9646f1ea-84a4-447b-8a9f-f5e16bc05e1e

📥 Commits

Reviewing files that changed from the base of the PR and between 8f45ecd and 0914049.

📒 Files selected for processing (35)
  • .pre-commit-config.yaml
  • src/aiida/cmdline/commands/cmd_node.py
  • src/aiida/cmdline/commands/cmd_process.py
  • src/aiida/engine/processes/workchains/restart.py
  • src/aiida/orm/nodes/data/array/bands.py
  • src/aiida/orm/nodes/data/structure.py
  • src/aiida/orm/utils/builders/code.py
  • src/aiida/plugins/entry_point.py
  • src/aiida/schedulers/plugins/pbsbaseclasses.py
  • src/aiida/storage/psql_dos/migrations/utils/provenance_redesign.py
  • src/aiida/storage/psql_dos/migrations/utils/utils.py
  • src/aiida/storage/psql_dos/orm/querybuilder/main.py
  • src/aiida/storage/psql_dos/orm/utils.py
  • src/aiida/tools/data/array/kpoints/legacy.py
  • src/aiida/tools/dbimporters/plugins/nninc.py
  • tests/cmdline/commands/test_node.py
  • tests/common/test_extendeddicts.py
  • tests/common/test_hashing.py
  • tests/engine/test_process.py
  • tests/orm/nodes/data/test_upf.py
  • tests/orm/nodes/test_node.py
  • tests/orm/test_groups.py
  • tests/orm/test_querybuilder.py
  • tests/orm/utils/test_managers.py
  • tests/storage/psql_dos/test_schema.py
  • tests/test_dataclasses.py
  • tests/test_nodes.py
  • tests/tools/archive/orm/test_computers.py
  • tests/tools/archive/orm/test_links.py
  • tests/tools/dumping/utils.py
  • tests/tools/graph/test_age.py
  • tests/tools/graph/test_graph_traversers.py
  • tests/tools/visualization/test_graph.py
  • tests/transports/test_all_plugins.py
  • utils/autogenerate_all_imports.py

📝 Walkthrough

Walkthrough

The pull request adds the pyupgrade pre-commit hook for Python 3.10+ syntax and applies equivalent syntax modernizations across production code, utilities, and tests.

Changes

Python syntax modernization

Layer / File(s) Summary
Source syntax and hook updates
.pre-commit-config.yaml, src/aiida/..., utils/autogenerate_all_imports.py
Adds the pyupgrade hook with --py310-plus. Updates set, dictionary, tuple, and generator expressions, plus exception handling and generator delegation.
Test syntax updates
tests/...
Updates test fixtures and assertions to use set literals, set comprehensions, generator expressions, and zero-argument super() calls. Expected behavior remains unchanged.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • aiidateam/aiida-core#7254: Both changes modernize Python syntax, but that PR enables Ruff UP rules and performs broader migrations.

Suggested reviewers: geigerj2, danielhollas

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 90.59% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title refers to linting rules, which relates to the added pyupgrade pre-commit hook but does not name the specific tool.
Description check ✅ Passed The description explains the addition of pyupgrade to prevent formatting and modernization changes from regressing.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.67%. Comparing base (2f36f79) to head (6969515).

Files with missing lines Patch % Lines
src/aiida/tools/data/array/kpoints/legacy.py 83.34% 2 Missing ⚠️
src/aiida/cmdline/commands/cmd_node.py 0.00% 1 Missing ⚠️
src/aiida/restapi/translator/nodes/data/kpoints.py 0.00% 1 Missing ⚠️
...e/psql_dos/migrations/utils/provenance_redesign.py 0.00% 1 Missing ⚠️
...c/aiida/storage/psql_dos/migrations/utils/utils.py 0.00% 1 Missing ⚠️
src/aiida/tools/dbimporters/plugins/nninc.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7543      +/-   ##
==========================================
- Coverage   80.68%   80.67%   -0.00%     
==========================================
  Files         581      581              
  Lines       47068    47068              
==========================================
- Hits        37971    37967       -4     
- Misses       9097     9101       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@danielhollas

Copy link
Copy Markdown
Collaborator

Most of the pyupgrade rules should be part of ruff, I don't think it's worth it adding an extra linter. Perhaps this could be a one-off PR without adding the hook?

It's possible that some of the changes in this PR are not in the UP category, but in a different category that we don't have activated in ruff at the moment.

@agoscinski
agoscinski force-pushed the pyupgrade-precommit branch from 0914049 to 358f70b Compare August 13, 2026 09:08
@agoscinski

agoscinski commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

Right thanks, we already have UP. It seems ruff also does it smarter than pyupgrade and excludes certain rules that might introduce performance issues UP038). pyupgrade does apply also the C4 rules (flake8-comprehensions) which included here now. This removes some unnecessary generator creations. One however cannot fix these automatically safely so this might be a argument to not add it @danielhollas @GeigerJ2 we can also close the PR whatever preferred.

@agoscinski
agoscinski force-pushed the pyupgrade-precommit branch from 358f70b to 2a1bbf8 Compare August 13, 2026 09:10
@agoscinski agoscinski changed the title Add pyupgrade as pre-commit hook Add C4 linting rule Aug 13, 2026
@agoscinski
agoscinski force-pushed the pyupgrade-precommit branch 3 times, most recently from 9795bce to 004c65f Compare August 13, 2026 10:07

@danielhollas danielhollas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

C4 rules are enabled by default in ruff 0.16 so it's fine to enable them here, thanks!

@agoscinski
agoscinski force-pushed the pyupgrade-precommit branch from 004c65f to b4818fa Compare August 13, 2026 10:14
These rules avoid creation on unnecessary generators.
@agoscinski
agoscinski force-pushed the pyupgrade-precommit branch from b4818fa to 6969515 Compare August 13, 2026 10:14
@agoscinski

Copy link
Copy Markdown
Collaborator Author

I separated in two commits, so one easier review the manual fixes in the second commit

@agoscinski agoscinski changed the title Add C4 linting rule Add C4 linting rules Aug 13, 2026
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.

2 participants