-
Notifications
You must be signed in to change notification settings - Fork 196
chore:migrate form black to ruff(#916) #1012
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request migrates the project from Black and isort to Ruff, consolidating linting and formatting. No security vulnerabilities were found. However, a critical issue was identified in the check/format-incremental script where a variable is used without being defined. Additionally, there's a potential regression in the isort configuration within pyproject.toml and a minor style guide violation in the new ruff.json problem matcher. Please refer to the detailed comments for specific suggestions.
| # Run ruff for import sorting. | ||
| # We do this first so that if it changes files, the formatting step will pick them up. | ||
| echo "Running ruff check (isort)..." | ||
| check_args=("check" "--select" "I" "${arg_color[@]}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The arg_color variable is used in the ruff commands on lines 118 and 137, but it is no longer defined in the script. The block of code that defined it was removed as part of this change. This will likely cause the script to fail or not produce colored output as intended. The definition should be restored.
# Color the output if it goes to a terminal or GitHub Actions log.
arg_color=()
if [[ -t 1 || "${CI}" == true ]]; then
arg_color=("--color")
fi
# Run ruff for import sorting.
# We do this first so that if it changes files, the formatting step will pick them up.
echo "Running ruff check (isort)..."
check_args=("check" "--select" "I" "${arg_color[@]}")
| ] | ||
| } | ||
| ] | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file is missing a final newline. The repository style guide requires files to end with a newline.
| } | |
| } | |
References
- Files must end with a final newline, unless they are special files that do not normally have ending newlines. (link)
352dec1 to
5b66e74
Compare
5b66e74 to
a4c1bc7
Compare
This PR transitions the qsim repository from Black to Ruff. This consolidates formatting, linting, and import sorting into a single, high-performance tool, aligning qsim with the standards used in related projects like Cirq.
Main Changes
Configuration:
Updated pyproject.toml to remove [tool.black] and [tool.isort] configurations.
Added [tool.ruff] targeting Python 3.10, including specific isort settings (combine-as-imports, known-first-party).
Development Environment:
Swapped black for ruff in dependencies.
Updated .pre-commit-config.yaml to use Ruff hooks.
Scripts & Automation:
Refactored check/format-incremental to utilize ruff check --fix and ruff format.
CI/CD & UX:
Replaced the legacy black.json problem matcher with a new ruff.json matcher.
Updated .github/workflows/ci.yaml to register the Ruff matcher, ensuring linting errors are annotated directly in the GitHub PR UI.
Verification Performed
Linting/Formatting: Ran check/format-incremental locally; the codebase is now 100% Ruff-compliant.
Package Integrity: Successfully ran python -m pytest --collect-only.
Result: 102 tests collected. This confirms that the reformatting and import sorting did not introduce syntax errors or break the package structure.
CI Logic: Verified the regex in ruff.json matches the output format of the astral-sh/ruff-action.