Project-MONAI
Replace `pyupgrade` with builtin Ruff's UP rule
simplify the lintong and reduce the number of used tools to increase consistency without any formatting loss
Description
A few sentences describing the changes proposed in this pull request.
Types of changes
- [x] Non-breaking change (fix or new feature that would not break existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing functionality to change).
- [ ] New tests added to cover the changes.
- [ ] Integration tests passed locally by running
./runtests.sh -f -u --net --coverage. - [ ] Quick tests passed locally by running
./runtests.sh --quick --unittests --disttests. - [ ] In-line docstrings updated.
- [ ] Documentation updated, tested
make htmlcommand in thedocs/folder.
[!WARNING]
Rate limit exceeded
@Borda has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 35 seconds before requesting another review.
⌛ How to resolve this issue?
After the wait time has elapsed, a review can be triggered using the
@coderabbitai reviewcommand as a PR comment. Alternatively, push new commits to this PR.We recommend that you space out your commits to avoid hitting the rate limit.
🚦 How do rate limits work?
CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.
📥 Commits
Reviewing files that changed from the base of the PR and between 5ba0ed1cb7615b5e1a3d3dd78524c415196bb761 and 623865492618d4a1c787809d9ce269ba0790a58f.
📒 Files selected for processing (35)
.pre-commit-config.yaml(1 hunks)monai/apps/detection/utils/anchor_utils.py(3 hunks)monai/losses/ds_loss.py(1 hunks)monai/losses/perceptual.py(1 hunks)monai/metrics/utils.py(2 hunks)monai/networks/blocks/attention_utils.py(1 hunks)monai/networks/blocks/crossattention.py(2 hunks)monai/networks/blocks/denseblock.py(1 hunks)monai/networks/blocks/mlp.py(1 hunks)monai/networks/blocks/patchembedding.py(1 hunks)monai/networks/blocks/pos_embed_utils.py(2 hunks)monai/networks/blocks/rel_pos_embedding.py(2 hunks)monai/networks/blocks/selfattention.py(3 hunks)monai/networks/blocks/spatialattention.py(1 hunks)monai/networks/blocks/transformerblock.py(1 hunks)monai/networks/layers/simplelayers.py(1 hunks)monai/networks/layers/utils.py(1 hunks)monai/networks/layers/vector_quantizer.py(4 hunks)monai/networks/nets/ahnet.py(1 hunks)monai/networks/nets/autoencoderkl.py(2 hunks)monai/networks/nets/basic_unet.py(1 hunks)monai/networks/nets/dints.py(4 hunks)monai/networks/nets/dynunet.py(5 hunks)monai/networks/nets/netadapter.py(2 hunks)monai/networks/nets/quicknat.py(3 hunks)monai/networks/nets/resnet.py(1 hunks)monai/networks/nets/segresnet_ds.py(3 hunks)monai/networks/nets/senet.py(1 hunks)monai/networks/nets/spade_network.py(3 hunks)monai/networks/nets/swin_unetr.py(1 hunks)monai/networks/nets/vista3d.py(6 hunks)monai/networks/nets/vqvae.py(3 hunks)monai/networks/schedulers/rectified_flow.py(1 hunks)monai/networks/trt_compiler.py(6 hunks)monai/networks/utils.py(1 hunks)
[!NOTE]
Other AI code review bot(s) detected
CodeRabbit 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.
Walkthrough
The diff applies widespread non-functional modernizations: many type hints converted from typing.Union/Optional and typing container aliases to PEP 604 (X | Y) and built-in generics (list[int], tuple[...]); several log/warning/error messages reformatted to f-strings; functools.lru_cache replaced with functools.cache in one metrics utility; pre-commit config adjusted (removed pyupgrade hook block, reformatted ruff args) and pyproject.toml enabled Ruff UP. No algorithmic changes or behavior-altering logic edits were introduced.
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
- Verify PEP 604 annotations and removed typing imports across losses, transforms, network modules, and utilities.
- Check public API signatures where annotations changed (nnUNet wrappers, SURELoss, DeepSupervisionLoss, vector quantizers, various network constructors/methods).
- Confirm f-string replacements preserve exact log/warning/error text and spacing expected by tests.
- Review functools.lru_cache → functools.cache semantic suitability for _get_neighbour_code_to_normals_table.
- Inspect pre-commit and pyproject.toml edits for intended hook/lint behavior.
Pre-merge checks and finishing touches
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | ⚠️ Warning | Description is incomplete. Missing issue reference, and explanation is minimal. Only vague mention of 'linting' without clearly explaining the pyupgrade-to-Ruff-UP migration or scope of changes. | Add issue reference (Fixes #...), clarify the pyupgrade replacement rationale, and explain why extensive typing/f-string changes accompany the tooling switch. |
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 73.33% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | Title clearly summarizes the main change: replacing pyupgrade tool with Ruff's UP rule for linting. |
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot] avatar](https://avatars.githubusercontent.com/in/347564?v=4 "Published by a pub.dev verified publisher"){kind=small}
Hi @Borda thanks for the contribution. There's a lot of changes related to typing and f-strings rather than the PyUpgrade change, these are fine but it obscures what is being done here. What is the purpose of changing the .pre-commit-config.yaml file? What is accomplished here? If you can look at the code format issue the changes are otherwise ready to go and be finalised.
There's a lot of changes related to typing and f-strings rather than the PyUpgrade change
Good call, but f-string is to be upgraded, and if original pyupgrade mised it, it is rather its issue than Ruff's doing its job, right?
What is the purpose of changing the
.pre-commit-config.yamlfile?
removing pyupgrade hooks, which become redundant
If you can look at the code format issue the changes are otherwise ready to go and be finalised.
Yes, will do
There's a few things coderabbit has mentioned that are worth looking at, but the failed checks are all related to versioneer files that should be ignored I think. I'm not sure why from your changes why these aren't being ignored now. All other changes looked fine to me however.