feat: replace mypy and pytype with pyrefly for static type analysis#8868
feat: replace mypy and pytype with pyrefly for static type analysis#8868garciadias wants to merge 17 commits into
Conversation
Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR replaces pytype/mypy with pyrefly across CI and developer tooling: workflow matrices, runtests.sh, requirements-dev, pyproject.toml, setup.cfg, .gitignore, and CONTRIBUTING.md were updated. It also adds many non-functional inline "# pyrefly: ignore[...]" annotations across source files to silence pyrefly diagnostics; one runtime behavioral change ensures a dict type check before an AMP-era update in EnsembleEvaluator._iteration. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
runtests.sh (1)
667-668: ⚡ Quick winQuote variables to prevent word splitting.
Shellcheck warns about unquoted variables that could cause issues with paths containing spaces.
🛡️ Proposed fix
- ${cmdPrefix}"${PY_EXE}" -m ty --version - ${cmdPrefix}"${PY_EXE}" -m ty check "$homedir" + ${cmdPrefix}"${PY_EXE}" -m ty --version + ${cmdPrefix}"${PY_EXE}" -m ty check "$homedir"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runtests.sh` around lines 667 - 668, The shell variables are unquoted and may split if they contain spaces; update the two invocations to quote variables: use "${cmdPrefix}${PY_EXE}" -m ty --version and "${cmdPrefix}${PY_EXE}" -m ty check "$homedir" (i.e., quote cmdPrefix and PY_EXE together and quote homedir) so that cmdPrefix, PY_EXE and homedir are not subject to word-splitting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@requirements-dev.txt`:
- Line 25: Pin the unversioned dev dependency "ty" in requirements-dev.txt to
avoid accidental breaking upgrades; update the "ty" entry to include a concrete
version or range (for example add a fixed version like ty==X.Y.Z or a bounded
range like ty>=1.0.0,<2.0.0) so it follows the pinned pattern used for other dev
deps (e.g., mypy) and prevents CI breakages.
---
Nitpick comments:
In `@runtests.sh`:
- Around line 667-668: The shell variables are unquoted and may split if they
contain spaces; update the two invocations to quote variables: use
"${cmdPrefix}${PY_EXE}" -m ty --version and "${cmdPrefix}${PY_EXE}" -m ty check
"$homedir" (i.e., quote cmdPrefix and PY_EXE together and quote homedir) so that
cmdPrefix, PY_EXE and homedir are not subject to word-splitting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8440b80a-96b0-4f0a-a30c-b8c8fc06909e
📒 Files selected for processing (8)
.github/workflows/pythonapp.yml.github/workflows/weekly-preview.yml.gitignoreCONTRIBUTING.mdpyproject.tomlrequirements-dev.txtruntests.shsetup.cfg
💤 Files with no reviewable changes (1)
- setup.cfg
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pyproject.toml (2)
101-104: 💤 Low valueRedundant override.
These files are already excluded in
tool.ty.src(lines 60-65). This override has no effect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` around lines 101 - 104, The override block under [tool.ty.overrides] that sets include = ["versioneer.py", "monai/_version.py"] and rules all = "ignore" is redundant because those files are already excluded in the existing [tool.ty.src] configuration; remove the entire [[tool.ty.overrides]] override (the include and rules entries) to avoid dead configuration and keep only the src exclusions.
71-99: ⚖️ Poor tradeoffAll type checking suppressed.
Every meaningful rule is set to "ignore," including critical checks (division-by-zero, call-non-callable, invalid-return-type). While matching mypy's baseline is the stated goal, consider enabling a minimal set of rules immediately to provide value during migration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` around lines 71 - 99, The config in pyproject.toml currently sets every type-check rule to "ignore", effectively disabling all checks; update this file to re-enable a minimal, high-value rule set instead of blanket ignores—at minimum change keys like invalid-return-type, call-non-callable, division-by-zero, unresolved-import, possibly-missing-attribute and unused-ignore-comment from "ignore" to "error" or "warning" so you get actionable feedback during migration; locate the block with those keys in pyproject.toml and set the selected rules to appropriate non-ignore levels (warning/error) and keep any remaining noisy rules ignored until you progressively enable them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyproject.toml`:
- Around line 68-70: Update the misleading comment about tool.ty.rules in
pyproject.toml: change the wording to state that [tool.ty.rules] only sets
severities for the listed rules and that unlisted rules keep their normal
defaults, and note that the "all = \"ignore\"" behavior applies only under
[tool.ty.overrides.rules] (used for versioneer.py / monai/_version.py), so
replace the current inaccurate lines with a concise comment reflecting these
facts and referencing [tool.ty.rules] and [tool.ty.overrides.rules].
---
Nitpick comments:
In `@pyproject.toml`:
- Around line 101-104: The override block under [tool.ty.overrides] that sets
include = ["versioneer.py", "monai/_version.py"] and rules all = "ignore" is
redundant because those files are already excluded in the existing [tool.ty.src]
configuration; remove the entire [[tool.ty.overrides]] override (the include and
rules entries) to avoid dead configuration and keep only the src exclusions.
- Around line 71-99: The config in pyproject.toml currently sets every
type-check rule to "ignore", effectively disabling all checks; update this file
to re-enable a minimal, high-value rule set instead of blanket ignores—at
minimum change keys like invalid-return-type, call-non-callable,
division-by-zero, unresolved-import, possibly-missing-attribute and
unused-ignore-comment from "ignore" to "error" or "warning" so you get
actionable feedback during migration; locate the block with those keys in
pyproject.toml and set the selected rules to appropriate non-ignore levels
(warning/error) and keep any remaining noisy rules ignored until you
progressively enable them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 198fbb14-ec24-4164-a127-1f82f828508d
📒 Files selected for processing (1)
pyproject.toml
| not-subscriptable = "ignore" | ||
| parameter-already-assigned = "ignore" | ||
| unsupported-operator = "ignore" | ||
| deprecated = "ignore" |
There was a problem hiding this comment.
Should we be ignoring this one, or does it give incorrect feedback? I'd like to minimise how many things we're ignoring so we don't hide real issues like we have in the past.
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com> Signed-off-by: Rafael Garcia-Dias <rafaelagd@gmail.com>
Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com> Signed-off-by: Rafael Garcia-Dias <rafaelagd@gmail.com>
- Remove [mypy] config from setup.cfg (migrated to [tool.pyrefly]) - Remove [tool.pytype] from pyproject.toml (deprecated, no Python >3.12) - Add [tool.pyrefly] with preset="legacy" matching mypy's laxness - Run pyrefly suppress to establish zero-error baseline (55 suppressed) - Update CI matrix: pytype + mypy → pyrefly - Update runtests.sh: --pytype + --mypy → --pyrefly - Update requirements-dev.txt, .gitignore, CONTRIBUTING.md - Closes Project-MONAI#8865 (pytype deprecation) Pyrefly is 15x faster than mypy, adopted by PyTorch, and auto-migrates existing mypy configuration.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/pythonapp.yml (1)
25-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit least-privilege workflow permissions.
This workflow still inherits the default
GITHUB_TOKENscope. For premerge CI, that is broader than necessary and weakens the security posture of the pipeline.Suggested fix
name: premerge on: # quick tests for pull requests and the releasing branches push: @@ pull_request: head_ref-ignore: - dev + +permissions: + contents: read concurrency:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pythonapp.yml around lines 25 - 55, Add an explicit least-privilege permissions block to the workflow (the flake8-py3 job) so the job no longer inherits the full GITHUB_TOKEN scope; insert a top-level permissions: map (e.g., permissions: contents: read) that grants only the required scopes for linting/checkout/cache operations and remove any implicit broader access—update the workflow YAML near the flake8-py3 job definition to include this permissions block so tools using GITHUB_TOKEN (actions/checkout, actions/cache, etc.) only get minimal read access..github/workflows/weekly-preview.yml (1)
12-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLock down the workflow token permissions.
This workflow still relies on default
GITHUB_TOKENpermissions. That is broader than needed for a lint/type-check pipeline and increases the blast radius if an action or dependency is compromised.Suggested fix
name: weekly-preview on: schedule: - cron: "0 2 * * 0" # 02:00 of every Sunday + +permissions: + contents: read jobs:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/weekly-preview.yml around lines 12 - 38, Add an explicit, minimal permissions block to the workflow to avoid using broad default GITHUB_TOKEN scopes: replace implicit defaults by adding a top-level permissions: block (e.g. permissions: contents: read, packages: read if needed) so the job only has the least privileges required for lint/type-check; reference the existing use of GITHUB_TOKEN by securing it with the permissions: stanza and ensure steps like actions/checkout@v4 and the pip cache/install steps will still work under those reduced scopes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@runtests.sh`:
- Around line 65-66: The help and option parsing treat -f/--codeformat as
running style checks but do not enable the pyrefly/type checker; update the
option handling so that the -f|--codeformat branch sets doPyreflyFormat=true (in
addition to any existing doCodeFormat/doClangFormat flags) and ensure the
usage/help echoes (the echo lines that list options) reflect that
-f/--codeformat includes pyrefly/type checking; search for the option parsing
block that handles -f/--codeformat and the variable doPyreflyFormat to make the
change and also update the repeated help strings mentioned (around the echoed
option lists) so the documentation matches behavior.
- Around line 315-319: The case branch for the `--mypy` option contains a stray
duplicate body (`doMypyFormat=true` and `;;`) that is unattached and breaks the
surrounding `case` block; remove the extra duplicate lines so the `--mypy)`
branch only appears once with a single `doMypyFormat=true` assignment and
terminating `;;`, ensuring there are no orphan `doMypyFormat=true` or `;;`
entries outside a `case` label.
---
Outside diff comments:
In @.github/workflows/pythonapp.yml:
- Around line 25-55: Add an explicit least-privilege permissions block to the
workflow (the flake8-py3 job) so the job no longer inherits the full
GITHUB_TOKEN scope; insert a top-level permissions: map (e.g., permissions:
contents: read) that grants only the required scopes for linting/checkout/cache
operations and remove any implicit broader access—update the workflow YAML near
the flake8-py3 job definition to include this permissions block so tools using
GITHUB_TOKEN (actions/checkout, actions/cache, etc.) only get minimal read
access.
In @.github/workflows/weekly-preview.yml:
- Around line 12-38: Add an explicit, minimal permissions block to the workflow
to avoid using broad default GITHUB_TOKEN scopes: replace implicit defaults by
adding a top-level permissions: block (e.g. permissions: contents: read,
packages: read if needed) so the job only has the least privileges required for
lint/type-check; reference the existing use of GITHUB_TOKEN by securing it with
the permissions: stanza and ensure steps like actions/checkout@v4 and the pip
cache/install steps will still work under those reduced scopes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be396dbe-9bae-4884-a142-5fa02c89b959
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (90)
.github/workflows/pythonapp.yml.github/workflows/weekly-preview.yml.gitignoreCONTRIBUTING.mdmonai/apps/auto3dseg/auto_runner.pymonai/apps/auto3dseg/bundle_gen.pymonai/apps/auto3dseg/ensemble_builder.pymonai/apps/deepedit/transforms.pymonai/apps/deepgrow/dataset.pymonai/apps/deepgrow/transforms.pymonai/apps/detection/networks/retinanet_detector.pymonai/apps/detection/transforms/array.pymonai/apps/detection/transforms/box_ops.pymonai/apps/detection/transforms/dictionary.pymonai/apps/detection/utils/hard_negative_sampler.pymonai/apps/generation/maisi/networks/autoencoderkl_maisi.pymonai/apps/nnunet/nnunetv2_runner.pymonai/apps/nnunet/utils.pymonai/apps/nuclick/transforms.pymonai/apps/pathology/transforms/post/dictionary.pymonai/apps/tcia/utils.pymonai/apps/vista3d/inferer.pymonai/apps/vista3d/transforms.pymonai/auto3dseg/operations.pymonai/auto3dseg/seg_summarizer.pymonai/bundle/reference_resolver.pymonai/bundle/scripts.pymonai/bundle/workflows.pymonai/config/__init__.pymonai/data/box_utils.pymonai/data/dataset.pymonai/data/folder_layout.pymonai/data/grid_dataset.pymonai/data/meta_tensor.pymonai/data/utils.pymonai/data/wsi_datasets.pymonai/data/wsi_reader.pymonai/engines/evaluator.pymonai/engines/trainer.pymonai/fl/client/monai_algo.pymonai/handlers/mlflow_handler.pymonai/handlers/utils.pymonai/inferers/inferer.pymonai/inferers/utils.pymonai/losses/dice.pymonai/losses/image_dissimilarity.pymonai/losses/multi_scale.pymonai/losses/tversky.pymonai/metrics/__init__.pymonai/metrics/utils.pymonai/networks/blocks/attention_utils.pymonai/networks/blocks/dints_block.pymonai/networks/blocks/localnet_block.pymonai/networks/blocks/squeeze_and_excitation.pymonai/networks/layers/spatial_transforms.pymonai/networks/nets/efficientnet.pymonai/networks/nets/flexible_unet.pymonai/networks/nets/milmodel.pymonai/networks/nets/swin_unetr.pymonai/networks/nets/transchex.pymonai/networks/nets/vista3d.pymonai/networks/nets/vqvae.pymonai/networks/utils.pymonai/optimizers/lr_scheduler.pymonai/optimizers/novograd.pymonai/transforms/compose.pymonai/transforms/croppad/array.pymonai/transforms/croppad/dictionary.pymonai/transforms/croppad/functional.pymonai/transforms/intensity/array.pymonai/transforms/io/dictionary.pymonai/transforms/lazy/utils.pymonai/transforms/spatial/array.pymonai/transforms/spatial/functional.pymonai/transforms/transform.pymonai/transforms/utility/array.pymonai/transforms/utility/dictionary.pymonai/transforms/utils.pymonai/transforms/utils_pytorch_numpy_unification.pymonai/utils/dist.pymonai/utils/enums.pymonai/utils/misc.pymonai/utils/module.pymonai/utils/type_conversion.pymonai/visualize/img2tensorboard.pymonai/visualize/visualizer.pypyproject.tomlrequirements-dev.txtruntests.shsetup.cfg
💤 Files with no reviewable changes (1)
- setup.cfg
✅ Files skipped from review due to trivial changes (81)
- monai/apps/detection/utils/hard_negative_sampler.py
- monai/apps/detection/transforms/box_ops.py
- monai/losses/image_dissimilarity.py
- monai/optimizers/lr_scheduler.py
- monai/networks/blocks/attention_utils.py
- monai/networks/nets/vista3d.py
- monai/config/init.py
- monai/apps/pathology/transforms/post/dictionary.py
- monai/optimizers/novograd.py
- monai/losses/tversky.py
- monai/transforms/utility/array.py
- monai/visualize/img2tensorboard.py
- monai/networks/blocks/localnet_block.py
- monai/losses/dice.py
- monai/apps/generation/maisi/networks/autoencoderkl_maisi.py
- monai/auto3dseg/operations.py
- monai/metrics/utils.py
- monai/losses/multi_scale.py
- monai/apps/detection/transforms/array.py
- monai/auto3dseg/seg_summarizer.py
- monai/networks/utils.py
- monai/apps/nnunet/nnunetv2_runner.py
- monai/data/wsi_reader.py
- monai/utils/dist.py
- monai/inferers/utils.py
- monai/transforms/intensity/array.py
- monai/metrics/init.py
- monai/apps/deepgrow/dataset.py
- monai/apps/nnunet/utils.py
- monai/networks/blocks/squeeze_and_excitation.py
- CONTRIBUTING.md
- monai/networks/nets/efficientnet.py
- monai/data/folder_layout.py
- monai/bundle/reference_resolver.py
- monai/apps/vista3d/inferer.py
- monai/data/utils.py
- monai/networks/nets/flexible_unet.py
- monai/engines/trainer.py
- monai/apps/detection/transforms/dictionary.py
- monai/apps/detection/networks/retinanet_detector.py
- monai/handlers/utils.py
- monai/apps/deepedit/transforms.py
- monai/data/grid_dataset.py
- monai/networks/nets/vqvae.py
- monai/apps/auto3dseg/ensemble_builder.py
- monai/transforms/croppad/dictionary.py
- monai/transforms/io/dictionary.py
- monai/apps/tcia/utils.py
- monai/bundle/scripts.py
- monai/apps/deepgrow/transforms.py
- monai/networks/nets/swin_unetr.py
- monai/utils/type_conversion.py
- monai/data/wsi_datasets.py
- monai/transforms/utility/dictionary.py
- monai/transforms/lazy/utils.py
- monai/inferers/inferer.py
- monai/apps/auto3dseg/auto_runner.py
- monai/utils/misc.py
- monai/networks/nets/transchex.py
- monai/networks/blocks/dints_block.py
- monai/apps/vista3d/transforms.py
- monai/apps/auto3dseg/bundle_gen.py
- monai/data/box_utils.py
- monai/apps/nuclick/transforms.py
- monai/handlers/mlflow_handler.py
- monai/data/meta_tensor.py
- monai/fl/client/monai_algo.py
- monai/transforms/spatial/functional.py
- monai/utils/module.py
- monai/transforms/utils_pytorch_numpy_unification.py
- monai/transforms/compose.py
- monai/transforms/croppad/functional.py
- monai/data/dataset.py
- monai/visualize/visualizer.py
- monai/transforms/croppad/array.py
- monai/networks/nets/milmodel.py
- monai/utils/enums.py
- monai/transforms/spatial/array.py
- monai/bundle/workflows.py
- monai/transforms/utils.py
- monai/networks/layers/spatial_transforms.py
| echo " [--clangformat] [--precommit] [--pyrefly]" | ||
| echo " [--unittests] [--disttests] [--coverage] [--quick] [--min] [--net] [--build] [--list_tests]" |
There was a problem hiding this comment.
Keep --codeformat wired to the active type checker.
-f/--codeformat is still documented as running all style and static analysis checks, but it never sets doPyreflyFormat=true. The shorthand now skips type checking unless callers remember to add --pyrefly.
Suggested fix
-f|--codeformat)
doBlackFormat=true
doIsortFormat=true
doFlake8Format=true
# doPylintFormat=true # https://github.com/Project-MONAI/MONAI/issues/7094
doRuffFormat=true
+ doPyreflyFormat=true
doCopyRight=true
;;Also applies to: 90-91, 265-272, 312-313
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@runtests.sh` around lines 65 - 66, The help and option parsing treat
-f/--codeformat as running style checks but do not enable the pyrefly/type
checker; update the option handling so that the -f|--codeformat branch sets
doPyreflyFormat=true (in addition to any existing doCodeFormat/doClangFormat
flags) and ensure the usage/help echoes (the echo lines that list options)
reflect that -f/--codeformat includes pyrefly/type checking; search for the
option parsing block that handles -f/--codeformat and the variable
doPyreflyFormat to make the change and also update the repeated help strings
mentioned (around the echoed option lists) so the documentation matches
behavior.
Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com>
Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com>
Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com>
Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com>
Description
Replace both mypy and pytype with pyrefly for static type analysis.
Why pyrefly:
preset="legacy"matches mypy laxness for smooth migration# type: ignorecomments still respectedpyrefly initauto-migrates existing mypy configurationpyrefly suppressestablishes zero-error baseline instantlyChanges:
[mypy]config from setup.cfg (migrated to[tool.pyrefly])[tool.pytype]from pyproject.toml (deprecated, no Python >3.12)[tool.pyrefly]withpreset="legacy"matching mypy laxnesspyrefly suppressto establish zero-error baselineFixes #8865 (pytype deprecation)