[Lint] Migrate from flake8/isort to ruff for faster linting #613
Conversation
- Replace flake8 and isort with ruff in pre-commit configuration - Configure comprehensive ruff rule set including pycodestyle, Pyflakes, pyupgrade, flake8-bugbear, flake8-simplify, isort, flake8-comprehensions, flake8-type-checking, and flake8-commas - Enable automatic fixing with --fix and --unsafe-fixes flags - Update pre-commit-hooks to v6.0.0 and tj-actions/changed-files to v47.0.0 - Configure per-file ignores for __init__.py and evals/harness.py - Set line length to 127 characters and target Python 3.10+ This change significantly improves linting performance while maintaining code quality standards.
Summary of ChangesHello @zhiyuan1i, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the project's linting process by migrating from a multi-tool setup of Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughBump Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the project's linting setup from flake8 and isort to ruff, which is a great move for performance. The configuration in pyproject.toml is comprehensive. However, I've found a critical issue in the .pre-commit-config.yaml that will prevent the ruff hook from running correctly. I've also suggested an improvement to maintain coverage for debug statements.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pyproject.toml (2)
56-67: Optional: broaden rule coverage (W, N, ISC, RUF).Current set is solid. Consider adding:
- "W" (pycodestyle warnings)
- "N" (pep8-naming)
- "ISC" (implicit‑str‑concat)
- "RUF" (Ruff‑specific rules)
These catch naming issues, implicit concatenations, and Ruff best‑practices.
select = [ "E", # pycodestyle + "W", # pycodestyle warnings "F", # Pyflakes "UP", # pyupgrade "B", # flake8-bugbear "SIM", # flake8-simplify "I", # isort "C4", # flake8-comprehensions "TCH", # flake8-type-checking "COM", # flake8-commas "T", # flake8-debugger + "N", # pep8-naming + "ISC", # flake8-implicit-str-concat + "RUF", # Ruff-specific rules ]
68-74: Reassess global ignores; prefer targeted per‑file ignores.Project‑wide "E741" and "B023" can hide real issues. Consider limiting them to files that truly need them via per‑file‑ignores.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check-pt-python-compatibility
🔇 Additional comments (4)
pyproject.toml (4)
48-51: Ruff target and line length look consistent.py310 aligns with requires‑python; 127 is set explicitly.
52-54: Formatter config looks good.docstring code formatting enabled; no issues.
76-78: Per-file ignore path verified.evals/harness.pyexists, so the ignore will apply.
37-39: Verify isort’s known-first-party configuration
Ensure “fla” covers all your actual top-level Python packages (e.g., modules under src/ or dirs with init.py). Add any missing entries to[tool.ruff.lint.isort].known-first-party.
|
Merge this before proceeding to the next version. After merging this PR, nearly every file will need to be reformatted. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.pre-commit-config.yaml (1)
14-18: Fix Ruff hook config: invalid rev and hook id; add ruff-formatUse a valid ruff-pre-commit tag and the correct hook IDs; add ruff-format. Optionally include --unsafe-fixes to match PR description.
Apply this diff:
- - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.14.0 - hooks: - - id: ruff-check - args: [--fix, --exit-non-zero-on-fix] + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.5.0 + hooks: + - id: ruff + args: [--fix, --unsafe-fixes, --exit-non-zero-on-fix] + - id: ruff-formatWhat is the latest valid tag for https://github.com/astral-sh/ruff-pre-commit and the correct hook IDs?
🧹 Nitpick comments (1)
pyproject.toml (1)
68-74: Consider revisiting E741 ignore (optional)Ignoring ambiguous variable names (E741) can reduce code clarity. If possible, enable E741 and fix offenders over time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.pre-commit-config.yaml(1 hunks)pyproject.toml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check-pt-python-compatibility
🔇 Additional comments (5)
pyproject.toml (4)
37-39: isort integration via Ruff: good setupknown-first-party and force-sort-within-sections are reasonable defaults.
48-51: Core Ruff config LGTMTargeting py310 and line-length 127 align with PR goals.
52-54: Docstring code formatting enabledNice. Be aware this can reflow fenced code in docstrings; ensure doctests/docs still pass.
55-67: Rule set selection looks solid (includes debugger rules)Covers pycodestyle, Pyflakes, pyupgrade, bugbear, simplify, isort, comprehensions, type-checking, commas, and debugger (T). Addresses prior feedback about missing debugger checks.
.pre-commit-config.yaml (1)
3-3: pre-commit-hooks v6.0.0: OK
| [tool.ruff.lint.per-file-ignores] | ||
| "__init__.py" = ["F401"] | ||
| "fla/utils.py" = ["TC004"] | ||
| "evals/harness.py" = ["I", "TCH"] |
There was a problem hiding this comment.
Typo in per-file ignore code: use TCH004 (not TC004)
Ruff’s flake8-type-checking codes use the TCH prefix; TC004 won’t match and the ignore won’t apply.
Apply this diff:
[tool.ruff.lint.per-file-ignores]
"__init__.py" = ["F401"]
-"fla/utils.py" = ["TC004"]
+"fla/utils.py" = ["TCH004"]
"evals/harness.py" = ["I", "TCH"]🤖 Prompt for AI Agents
In pyproject.toml around lines 76 to 79, the per-file ignore for "fla/utils.py"
uses the incorrect code "TC004"; change it to the correct Ruff
flake8-type-checking code "TCH004" so the ignore actually applies, i.e., replace
"TC004" with "TCH004" in that entry.
1 participant
This change significantly improves linting performance while maintaining code quality standards.
Summary by CodeRabbit
No user-facing changes or functional impact.