-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Complete replace flake8 with ruff and update rules
#8694
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: dev
Are you sure you want to change the base?
Conversation
Signed-off-by: jirka <jirka.borovec@seznam.cz>
📝 WalkthroughWalkthroughThis PR migrates linting tooling from Flake8 to Ruff. Changes include: renaming GitHub Actions jobs from Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (7)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.py⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (1)monai/transforms/regularization/dictionary.py (1)
⏰ 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). (11)
🔇 Additional comments (3)
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 |
for more information, see https://pre-commit.ci
Signed-off-by: jirka <jirka.borovec@seznam.cz>
for more information, see https://pre-commit.ci
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.
Pull request overview
This pull request migrates the project's Python linting infrastructure from flake8 to ruff, consolidating all linting configuration into pyproject.toml and removing flake8-related dependencies and scripts.
Changes:
- Removed all flake8 and flake8-plugin dependencies from requirements-dev.txt
- Deleted the [flake8] section from setup.cfg
- Removed flake8-related flags, variables, and test sections from runtests.sh
- Expanded ruff configuration in pyproject.toml with additional rule sets (B, C90, N, PIE, TID) and migrated ignore rules
- Updated GitHub Actions workflow job names from
flake8-py3tolint-py3 - Changed ruff line-length from 133 to 120 to match black's configuration
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.cfg | Removed the [flake8] configuration section including select, ignore, and exclude patterns |
| runtests.sh | Removed flake8 command-line flags, shell variables, and the flake8 test execution block |
| requirements-dev.txt | Removed flake8 and its plugins (flake8-bugbear, flake8-comprehensions) from development dependencies |
| pyproject.toml | Expanded ruff lint rule selection and migrated flake8 ignore rules with documentation; adjusted line-length to 120 |
| .github/workflows/weekly-preview.yml | Renamed job from flake8-py3 to lint-py3 |
| .github/workflows/pythonapp.yml | Renamed job from flake8-py3 to lint-py3 |
Comments suppressed due to low confidence (1)
requirements-dev.txt:16
- The packages mccabe, pep8-naming, pycodestyle, and pyflakes are now redundant since ruff includes all their functionality. These were previously used by flake8 and its plugins, but since flake8 has been removed, these dependencies should also be removed unless they are used by another tool in the project (e.g., if yesqa in .pre-commit-config.yaml still needs them).
mccabe
pep8-naming
pycodestyle
pyflakes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pyproject.toml (2)
57-57: Consider enablingRUF100to catch stale noqa directives.With rule sets changing, some noqa comments may become obsolete. RUF100 auto-detects these.
Suggested change
"NPY", # NumPy specific rules "UP", # pyupgrade - # "RUF100", # aka yesqa + "RUF100", # aka yesqa - catches unused noqa directives ]
65-66: Commented rules W503/W504 don't exist in ruff.These were pycodestyle-only rules. Safe to remove the comments entirely.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (18)
.github/workflows/pythonapp.yml.github/workflows/weekly-preview.ymlmonai/apps/detection/transforms/box_ops.pymonai/auto3dseg/algo_gen.pymonai/data/box_utils.pymonai/fl/client/client_algo.pymonai/networks/blocks/feature_pyramid_network.pymonai/networks/nets/dints.pymonai/networks/trt_compiler.pymonai/transforms/traits.pymonai/utils/profiling.pypyproject.tomlrequirements-dev.txtruntests.shsetup.cfgtests/handlers/test_handler_regression_metrics.pytests/integration/test_integration_classification_2d.pytests/utils/type_conversion/test_convert_data_type.py
💤 Files with no reviewable changes (10)
- setup.cfg
- tests/integration/test_integration_classification_2d.py
- tests/utils/type_conversion/test_convert_data_type.py
- monai/auto3dseg/algo_gen.py
- requirements-dev.txt
- monai/networks/trt_compiler.py
- monai/transforms/traits.py
- monai/networks/blocks/feature_pyramid_network.py
- monai/fl/client/client_algo.py
- monai/networks/nets/dints.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/utils/profiling.pymonai/apps/detection/transforms/box_ops.pymonai/data/box_utils.pytests/handlers/test_handler_regression_metrics.py
🪛 Ruff (0.14.10)
monai/utils/profiling.py
340-340: Unused noqa directive (non-enabled: N805)
Remove unused noqa directive
(RUF100)
⏰ 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). (18)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: lint-py3 (mypy)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: packaging
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: lint-py3 (pytype)
- GitHub Check: lint-py3 (codeformat)
🔇 Additional comments (13)
monai/data/box_utils.py (4)
594-596: LGTM!Simplifying
range(0, spatial_dims)torange(spatial_dims)— idiomatic Python, no behavioral change.
734-736: LGTM!Same stylistic cleanup —
range(spatial_dims)is cleaner.
1044-1050: LGTM!Consistent with other range simplifications in this PR.
1136-1136: LGTM!Range simplification applied consistently.
monai/apps/detection/transforms/box_ops.py (1)
181-183: LGTM!Default
flip_axescomputation simplified. Same behavior.tests/handlers/test_handler_regression_metrics.py (1)
73-73: LGTM!Direct keyword argument is cleaner than dict unpacking.
pyproject.toml (2)
42-42: LGTM!Line length now consistent with black config (line 12).
47-57: Rule selection looks good.Comprehensive coverage migrating from flake8 plugins. Comments with doc links are helpful.
.github/workflows/weekly-preview.yml (1)
8-8: LGTM!Job name now accurately reflects it runs general linting, not just flake8.
.github/workflows/pythonapp.yml (1)
25-25: LGTM!Consistent with weekly-preview.yml rename.
monai/utils/profiling.py (1)
340-340: The# noqa: N805directive is necessary and should be retained.N805 is active in ruff configuration (N rules selected, N805 not ignored). The code intentionally uses
_selfto avoid shadowing the outerselfscope, making the noqa suppression valid.Likely an incorrect or invalid review comment.
runtests.sh (2)
62-62: LGTM:--flake8option removed from usage.Consistent with the PR objective. The help text now correctly reflects available linting options with ruff as the replacement.
265-271: LGTM:--codeformatnow relies solely on ruff for linting.The shorthand
-f|--codeformatcorrectly activates black, isort, ruff, and copyright checks. Flake8 reference removed as expected.
Signed-off-by: jirka <jirka.borovec@seznam.cz>
Signed-off-by: jirka <jirka.borovec@seznam.cz>
Signed-off-by: jirka <jirka.borovec@seznam.cz>
Signed-off-by: jirka <jirka.borovec@seznam.cz>
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pyproject.toml (1)
83-84: max-complexity = 50 is essentially disabled.Standard threshold is 10-15. The TODO acknowledges this needs lowering. Also, typo: "treshold" → "threshold".
Typo fix
-max-complexity = 50 # todo lower this treshold when yesqa id replaced with Ruff's RUF100 +max-complexity = 50 # todo lower this threshold when yesqa is replaced with Ruff's RUF100
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (8)
monai/apps/auto3dseg/bundle_gen.pymonai/metrics/confusion_matrix.pymonai/metrics/loss_metric.pymonai/transforms/regularization/array.pymonai/transforms/regularization/dictionary.pymonai/transforms/traits.pymonai/utils/deprecate_utils.pypyproject.toml
💤 Files with no reviewable changes (1)
- monai/transforms/traits.py
✅ Files skipped from review due to trivial changes (1)
- monai/metrics/confusion_matrix.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/transforms/regularization/dictionary.pymonai/apps/auto3dseg/bundle_gen.pymonai/metrics/loss_metric.pymonai/transforms/regularization/array.pymonai/utils/deprecate_utils.py
🧬 Code graph analysis (2)
monai/transforms/regularization/dictionary.py (1)
monai/transforms/transform.py (2)
MapTransform(372-497)RandomizableTransform(328-369)
monai/transforms/regularization/array.py (1)
monai/transforms/transform.py (1)
RandomizableTransform(328-369)
🔇 Additional comments (10)
monai/transforms/regularization/array.py (1)
22-22: LGTM!Absolute import aligns with PR's import normalization pattern. Path correctly references
monai.transforms.transform.monai/transforms/regularization/dictionary.py (1)
24-25: LGTM!Absolute import for
MapTransformandRandomizableTransformis consistent with sibling modulearray.py.monai/metrics/loss_metric.py (1)
22-23: LGTM!Absolute import for
TensorOrListfollows the PR's normalization pattern.monai/utils/deprecate_utils.py (2)
24-24: LGTM!Absolute import for
__version__follows the PR's normalization pattern.
203-207: LGTM!Using
_to discard the binding result suppresses unused-variable warnings from ruff while preserving the validation logic.monai/apps/auto3dseg/bundle_gen.py (1)
210-213: LGTM!Tuple-based
endswithis cleaner and idiomatic. Functionally equivalent to separate checks.pyproject.toml (4)
42-42: Good alignment with black.
line-length = 120matches the black configuration on line 12. Consistency achieved.
46-58: Rule expansion looks solid.Adding B, C90, N, PIE, TID covers the flake8 plugins being removed. RUF100 commented out with clear TODO intent.
63-70: Clarify E501 + line-length interaction.E501 is ignored, but
line-length = 120is set. Ruff usesline-lengthfor the formatter and E501 for the linter. Ignoring E501 means no lint warnings for long lines—only the formatter will wrap. If that's intentional (let formatter handle it), this is fine. Otherwise, remove E501 from ignore.
72-81: Per-file ignores are reasonable.Tests get flexibility on naming (N999, N801), complexity (C901), and useless expressions (B018). ATSS_matcher.py exception for N999 due to filename.
Signed-off-by: jirka <jirka.borovec@seznam.cz>
|
@ericspod as part of this PR, I have renamed the CI jobs stated as |
This pull request removes support for
flake8in favor of usingruffas the primary Python linter and code style checker. It updates configuration files, scripts, and workflow definitions to reflect this change, and consolidates all linting rules and ignores intopyproject.tomlunder the[tool.ruff]section.Migration from flake8 to ruff:
Removed all references to
flake8and its plugins fromrequirements-dev.txt,setup.cfg, and theruntests.shscript, makingruffthe sole linter for code style checks. [1] [2] [3] [4] [5] [6] [7] [8]Updated GitHub Actions workflow job names from
flake8-py3tolint-py3to reflect the switch toruff. [1] [2]Ruff configuration enhancements:
Expanded the
[tool.ruff.lint].selectlist inpyproject.tomlto include additional rule sets previously covered byflake8plugins (e.g.,Bfor flake8-bugbear,C90for mccabe,Nfor pep8-naming, etc.).Migrated and extended ignore rules from
flake8toruff, ensuring that all previously ignored warnings and errors are now managed throughpyproject.toml.Adjusted
line-lengthinpyproject.tomlfrom 133 to 120 to standardize formatting.Cleanup:
[flake8]section fromsetup.cfg, consolidating all linting configuration intopyproject.toml.