feat: replace mypy with ty for static type analysis #8868
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -116,6 +116,7 @@ examples/scd_lvsegs.npz | |
| temp/ | ||
| .idea/ | ||
| .dmypy.json | ||
| .ty_cache/ | ||
|
|
||
| *~ | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,7 +38,7 @@ Please note that, as per PyTorch, MONAI uses American English spelling. This mea | |
|
|
||
| ### Preparing pull requests | ||
| To ensure the code quality, MONAI relies on several linting tools ([flake8 and its plugins](https://gitlab.com/pycqa/flake8), [black](https://github.com/psf/black), [isort](https://github.com/timothycrosley/isort), [ruff](https://github.com/astral-sh/ruff)), | ||
| static type analysis tools ([ty](https://docs.astral.sh/ty/), [pytype](https://github.com/google/pytype)), as well as a set of unit/integration tests. | ||
|
|
||
| This section highlights all the necessary preparation steps required before sending a pull request. | ||
| To collaborate efficiently, please read through this section and follow them. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,57 @@ extend-ignore = [ | |
| "NPY002", # numpy-legacy-random | ||
| ] | ||
|
|
||
| [tool.ty] | ||
| [tool.ty.environment] | ||
| python-version = "3.9" | ||
|
|
||
| [tool.ty.src] | ||
| exclude = [ | ||
| "**/venv/**", | ||
| "**/.venv/**", | ||
| "versioneer.py", | ||
| "monai/_version.py", | ||
| ] | ||
|
|
||
| [tool.ty.rules] | ||
| # Only rules explicitly listed here; all unlisted rules are ignored. | ||
| # This provides a stable baseline matching mypy's output (zero diagnostics on monai/). | ||
| # Rules can be re-enabled incrementally as the codebase is improved. | ||
|
Comment on lines
+68
to
+70
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🌐 Web query:
💡 Result: Default behavior: if you configure Citations:
🏁 Script executed: #!/bin/bash
# Verify ty configuration syntax and default behavior
ty --help 2>&1 | head -20Repository: Project-MONAI/MONAI Length of output: 104 🏁 Script executed: #!/bin/bash
# Inspect tool.ty.rules config around the referenced lines
sed -n '1,120p' pyproject.toml | nl -ba | sed -n '1,120p'
rg -n "^\[tool\.ty\.rules\]" -n pyproject.toml || true
rg -n "tool\.ty\.rules|^\s*all\s*=" pyproject.toml || true
# Print the full rules block (up to the next [section])
awk '
BEGIN{inblock=0}
/^\[tool\.ty\.rules\]/{inblock=1; print; next}
inblock && /^\[/{inblock=0}
inblock{print}
' pyproject.toml | nl -baRepository: Project-MONAI/MONAI Length of output: 237 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Show the relevant ty config area with line numbers via `cat -n` (nl isn't available)
sed -n '55,115p' pyproject.toml | cat -n
# Extract just the tool.ty.rules block
awk '
/^\[tool\.ty\.rules\]/{inblock=1}
inblock{print}
inblock && /^\[/{ if($0 !~ /^\[tool\.ty\.rules\]/){exit} }
' pyproject.toml | cat -n
# List rule keys under tool.ty.rules (excluding the header line)
awk '
$0 ~ /^\[tool\.ty\.rules\]/{inblock=1; next}
inblock{
if($0 ~ /^\[/{exit}
if($0 ~ /^[[:space:]]*`#/` || $0 ~ /^[[:space:]]*$/) next
if($0 ~ /=/) print
}
' pyproject.toml | rg -n "^[0-9]+:.*=" || trueRepository: Project-MONAI/MONAI Length of output: 3829 🌐 Web query:
💡 Result: In ty, per-rule configuration works by merging configured rule severities from global rules ([tool.ty.rules] / [rules]) and then applying overrides; later overrides take precedence, and override rules “will be merged with the global rules” ([1], [2]). There’s no statement in the official configuration docs that “unlisted rules are ignored” or that fallback “per-rule default wording” is skipped when [tool.ty.rules] contains an incomplete set; instead, the documented model is that you can specify only the rules you care about, and the rest follow their normal configured defaults unless you disable them globally (for example via all = “ignore”) or enable/disable via CLI flags like --warn/--error/--ignore [1]. Your quoted behavior is more likely to be explained by how ty treats “type: ignore” codes vs “ty: ignore” codes, which can appear as “unlisted rules ignored” in practice. ty’s configuration includes respect-type-ignore-comments (default true): when false, ty treats type: ignore as a normal comment and “can’t be used to suppress ty errors” (you must use ty: ignore instead) [1]. Separately, ty has a rule ignore-comment-unknown-rule that warns when you use ty: ignore[code] or type: ignore[ty:code] with a code that isn’t a known lint rule, i.e., unrecognized codes in ignore comments won’t suppress anything and are likely a mistake [3]. So, based on the current official docs: (1) [tool.ty.rules] doesn’t imply that unlisted rules are ignored; it only sets severities for the rules you name, while other rules use their normal defaults unless you change them globally; and (2) apparent “ignoring fallback/default wording” is commonly due to suppression behavior (especially around type: ignore vs ty: ignore, and unknown ignore codes) rather than a bug in configuration merging [1][3]. Citations:
Fix misleading 🤖 Prompt for AI Agents |
||
| unresolved-import = "ignore" | ||
| unused-ignore-comment = "ignore" | ||
| unused-type-ignore-comment = "ignore" | ||
| unresolved-attribute = "ignore" | ||
| unresolved-reference = "ignore" | ||
| invalid-method-override = "ignore" | ||
| invalid-argument-type = "ignore" | ||
| invalid-assignment = "ignore" | ||
| invalid-return-type = "ignore" | ||
| invalid-type-form = "ignore" | ||
| invalid-declaration = "ignore" | ||
| invalid-yield = "ignore" | ||
| call-non-callable = "ignore" | ||
| call-top-callable = "ignore" | ||
| index-out-of-bounds = "ignore" | ||
| missing-argument = "ignore" | ||
| no-matching-overload = "ignore" | ||
| not-iterable = "ignore" | ||
| not-subscriptable = "ignore" | ||
| parameter-already-assigned = "ignore" | ||
| unsupported-operator = "ignore" | ||
| deprecated = "ignore" | ||
|
Member
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. |
||
| division-by-zero = "ignore" | ||
| invalid-enum-member-annotation = "ignore" | ||
| possibly-missing-attribute = "ignore" | ||
| possibly-missing-submodule = "ignore" | ||
| possibly-unresolved-reference = "ignore" | ||
| unknown-argument = "ignore" | ||
| too-many-positional-arguments = "ignore" | ||
|
|
||
| [[tool.ty.overrides]] | ||
| include = ["versioneer.py", "monai/_version.py"] | ||
| [tool.ty.overrides.rules] | ||
| all = "ignore" | ||
|
|
||
| [tool.pytype] | ||
| # Space-separated list of files or directories to exclude. | ||
| exclude = ["versioneer.py", "_version.py"] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ isort>=5.1, <6.0 | |
| ruff | ||
| pytype>=2020.6.1; platform_system != "Windows" | ||
| types-setuptools | ||
| ty>=0.0.38 | ||
| ninja | ||
| torchio | ||
| torchvision | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,7 @@ doClangFormat=false | |
| doCopyRight=false | ||
| doPytypeFormat=false | ||
| doMypyFormat=false | ||
| doTyFormat=false | ||
| doCleanup=false | ||
| doDistTests=false | ||
| doPrecommit=false | ||
|
|
@@ -61,7 +62,7 @@ PY_EXE=${MONAI_PY_EXE:-$(which python)} | |
|
|
||
| function print_usage { | ||
| echo "runtests.sh [--codeformat] [--autofix] [--black] [--isort] [--flake8] [--pylint] [--ruff]" | ||
| echo " [--clangformat] [--precommit] [--pytype] [-j number] [--mypy] [--ty]" | ||
| echo " [--unittests] [--disttests] [--coverage] [--quick] [--min] [--net] [--build] [--list_tests]" | ||
| echo " [--dryrun] [--copyright] [--clean] [--help] [--version] [--path] [--formatfix]" | ||
| echo "" | ||
|
|
@@ -90,6 +91,7 @@ function print_usage { | |
| echo " --pytype : perform \"pytype\" static type checks" | ||
| echo " -j, --jobs : number of parallel jobs to run \"pytype\" (default $NUM_PARALLEL)" | ||
| echo " --mypy : perform \"mypy\" static type checks" | ||
| echo " --ty : perform \"ty\" static type checks" | ||
| echo "" | ||
| echo "MONAI unit testing options:" | ||
| echo " -u, --unittests : perform unit testing" | ||
|
|
@@ -317,6 +319,9 @@ do | |
| --mypy) | ||
| doMypyFormat=true | ||
| ;; | ||
| --ty) | ||
| doTyFormat=true | ||
| ;; | ||
| -j|--jobs) | ||
| NUM_PARALLEL=$2 | ||
| shift | ||
|
|
@@ -679,6 +684,31 @@ then | |
| fi | ||
|
|
||
|
|
||
| if [ $doTyFormat = true ] | ||
| then | ||
| set +e # disable exit on failure so that diagnostics can be given on failure | ||
| echo "${separator}${blue}ty${noColor}" | ||
|
|
||
| # ensure that the necessary packages for code format testing are installed | ||
| if ! is_pip_installed ty | ||
| then | ||
| install_deps | ||
| fi | ||
| ${cmdPrefix}"${PY_EXE}" -m ty --version | ||
| ${cmdPrefix}"${PY_EXE}" -m ty check "$homedir" | ||
|
|
||
| ty_status=$? | ||
| if [ ${ty_status} -ne 0 ] | ||
| then | ||
| : # ty output already follows format | ||
| exit ${ty_status} | ||
| else | ||
| : # ty output already follows format | ||
| fi | ||
| set -e # enable exit on failure | ||
| fi | ||
|
|
||
|
|
||
| # testing command to run | ||
| cmd="${PY_EXE}" | ||
|
|
||
|
|
||
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.
What I want to do instead is replace
pytypewithtyand see how that works for us. There's a lot of errors it throws up when I initially tried it, but we can work through those and configure what to ignore. We can then see iftyprovides any value, otherwise we rely on mypy only. Since pytype is essentially deprecated now we should remove it either way from the test process but keep the script option to run it.