Skip to content

Conversation

pavoljuhas
Copy link
Collaborator

The check/format-incremental needs more work.
isort invocation should skip __init__.py files, but it does not.

Related to #7181

The `check/format-incremental` needs more work.
isort invocation should skip `__init__.py` files, but it does not.

Related to quantumlib#7181
@pavoljuhas pavoljuhas requested review from a team and vtomole as code owners March 26, 2025 20:40
isort "${args[@]}" "${format_files[@]}"
ISORTSTATUS=$?
echo "skipped"; ISORTSTATUS=0
# isort "${args[@]}" "${format_files[@]}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer not having commented code in script files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done in ac22abb

@pavoljuhas pavoljuhas enabled auto-merge March 26, 2025 20:48
@pavoljuhas
Copy link
Collaborator Author

NB: this is a quick fix to prevent isort misbehaving in CI checks.

To be reverted after fixups in check/format-incremental

@daxfohl
Copy link
Collaborator

daxfohl commented Mar 26, 2025

What happened? It worked for me when I added the skip directive to the config.

@pavoljuhas
Copy link
Collaborator Author

pavoljuhas commented Mar 26, 2025

What happened? It worked for me when I added the skip directive to the config.

The skip directive is overruled if such file is present in isort arguments.
This would happen for check/format-incremental --all or if some __init__.py is
changed in a PR.

The __init__.py files should be excluded from isort, but should stay present for black.

@daxfohl
Copy link
Collaborator

daxfohl commented Mar 26, 2025

Oh I see. The skip directive only works when calling isort . but the script specifies which files to isort explicitly, which seems to override the config.

@pavoljuhas pavoljuhas added this pull request to the merge queue Mar 26, 2025
Copy link

codecov bot commented Mar 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.14%. Comparing base (3b268c2) to head (ac22abb).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7190   +/-   ##
=======================================
  Coverage   98.14%   98.14%           
=======================================
  Files        1100     1100           
  Lines       96191    96191           
=======================================
  Hits        94409    94409           
  Misses       1782     1782           

☔ View full report in Codecov by Sentry.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Merged via the queue into quantumlib:main with commit 7c8f58f Mar 26, 2025
38 checks passed
@pavoljuhas pavoljuhas deleted the disable-isort branch March 26, 2025 21:21
BichengYing pushed a commit to BichengYing/Cirq that referenced this pull request Jun 20, 2025
* CI - skip sorting of imports with isort

The `check/format-incremental` needs more work.
isort invocation should skip `__init__.py` files, but it does not.

Related to quantumlib#7181

* Add TODO note for re-enabling isort
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.

4 participants