Add type-checking by szabgab · Pull Request #2174 · scverse/anndata

szabgab · 2025-10-25T08:56:26Z

Add .mypy.ini
Add mypy to pre-commit
Add mypy to GitHub Actioons

Starts working on Add type-annotation, enable mypy to check it #2173
Tests added
Release note added (or unnecessary)

ilan-gold · 2025-10-25T09:00:00Z

.github/workflows/mypy.yml

@@ -0,0 +1,26 @@
+name: Run mypy


I think we have a pre-commit bot that already runs on PRs, so in theory should be running the added mypy to the .pre-commit-config.yaml

There it is! https://results.pre-commit.ci/run/github/100038377/1761382588.jTYvjMUUTWKRTaM3owMDkw

You are right, Now I can see it is triggered in .github/workflows/check-pr.yml.
What is unclear to me why is it failing on the pre-commit.ci server. It seems that it does not take in account the .mypy.ini that excludes the duplicate files.

If I run pre-commit run -a locally, I get the same error as we can see on pre-commit.ci

mypy.....................................................................Failed - hook id: mypy - exit code: 2 src/anndata/_settings.pyi: error: Duplicate module named "anndata._settings" (also at "src/anndata/_settings.py") Found 1 error in 1 file (errors prevented further checking)

I've commented out mypy in the pre-commit hook to make ti pass on the pre-commit.ci server.
We still have it on GitHub Actions.
Either someone will figure out what's the problem on the pre-commit.ci server or this can be revisited later.

Re: what Lukas is saying in the review, it should probably be the reverse. We should figure out why things aren't working on the pre-commit ci instead of trying to use github actions.

Zethson · 2025-10-27T09:46:28Z

I don't understand why we would add a GHA workflow instead of just adding mypy to the pre-commit config. The latter should suffice.

I also suggest that we put the mypy config into the pyproject.toml file.

Thanks @szabgab for getting this started!

szabgab · 2025-10-27T13:33:33Z

I've moved the mypy configuration to pyproject.toml and rebased the PR.

I still could not figure out why does mypy fail on the pre-commit.ci while passes on GHA. That's the reason I kept GHA.

flying-sheep · 2025-10-28T15:41:53Z

I still could not figure out why does mypy fail on the pre-commit.ci while passes on GHA. That's the reason I kept GHA.

Because pre-commit creates isolated environments. You’d need to duplicate all dependencies in .pre-commit-config.yaml to make it work, like here:

https://github.com/scverse/fast-array-utils/blob/6af5d9a339a78604ff62230e6462f3a7a94fec3f/.pre-commit-config.yaml#L24-L41

* Add .mypy.ini * Add mypy to pre-commit * Add mypy to GitHub Actioons See scverse#2173

szabgab · 2025-10-29T15:19:17Z

It seems that rebasing to main broke mypy. Specifically this commit 470db9c
The class name[] syntax seems something that mypy does not accept.

flying-sheep · 2025-10-31T12:36:56Z

Starting with version 1.12, mypy supports this syntax: https://mypy.readthedocs.io/en/latest/changelog.html#support-python-3-12-syntax-for-generics-pep-695

I see an internal error, so it’s for sure a mypy bug though!

flying-sheep · 2025-10-31T12:50:13Z

Seems like it’s either fixed on master or the “49 files” mypy checks are docs and stuff. No way it just succeeds.

szabgab · 2025-10-31T13:25:55Z

So maybe you could go ahead and merge this now.
We might later move the mypy run to the pre-commit hook, though it seems it is also disabled on fast-array-utils

Zethson · 2025-11-25T11:59:14Z

.pre-commit-config.yaml

      - id: codespell
        additional_dependencies:
          - tomli
+# See https://github.com/scverse/anndata/pull/2174


We can also exclude them like this:

https://github.com/laminlabs/lamindb/blob/main/.pre-commit-config.yaml#L71

Zethson · 2025-11-25T11:59:50Z

.github/workflows/mypy.yml

@@ -0,0 +1,27 @@
+name: mypy


I still think that this whole workflow shouldn't exist and we can make it work with pre-commit. See below for the hammer - just exclude the unimportant conftest files.

Zethson · 2025-11-25T12:00:10Z

.github/workflows/mypy.yml

+          python-version: ${{ env.max_python_version }}
+      - name: Install mypy
+        run: |
+          pip install 'mypy @ git+https://github.com/python/mypy.git'


Even if we were to use this workflow, we shouldn't install from Github. We can just use a release.

Zethson · 2025-11-25T12:00:26Z

.pre-commit-config.yaml

@@ -1,5 +1,6 @@
 ci:
  autoupdate_commit_msg: "ci: pre-commit autoupdate"
+  skip: [mypy]  # too big


Suggested change

skip: [mypy] # too big

@Zethson did you test that it works? pre-commit.ci often cries when the mypy environment contains a halfway useful subset of a package’s dependencies, so I basically have to add this skip to almost everything.

No I did not but I have never had to use it anywhere. But you're probably right then

ilan-gold reviewed Oct 25, 2025

View reviewed changes

szabgab force-pushed the add-types branch from a7aa494 to c62ae4d Compare October 27, 2025 13:24

szabgab added 6 commits October 29, 2025 16:44

Add type-checking

c2c517d

* Add .mypy.ini * Add mypy to pre-commit * Add mypy to GitHub Actioons See scverse#2173

exclude less files by mypy

d01dbda

comment out mypy in the pre-commit hook for now

5558e9e

make name of the ci job stand out

da08894

move mypy configuration to pyproject.toml

94bff07

try to add requiremens to pre-commit hook

f17d1b4

szabgab force-pushed the add-types branch from fcc2dcc to f17d1b4 Compare October 29, 2025 14:48

disable mypy pre-commit on the ci as it is too big

bf2d18a

flying-sheep added 5 commits October 31, 2025 13:37

add --show-traceback

e692b90

use max Python version

91ed109

fix pre-commit

40c895f

use git mypy as instructed

b067c8e

oops

aa1c237

Zethson requested changes Nov 25, 2025

View reviewed changes

Conversation

szabgab commented Oct 25, 2025

Uh oh!

ilan-gold Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zethson commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szabgab commented Oct 27, 2025

Uh oh!

flying-sheep commented Oct 28, 2025

Uh oh!

szabgab commented Oct 29, 2025

Uh oh!

flying-sheep commented Oct 31, 2025

Uh oh!

flying-sheep commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szabgab commented Oct 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zethson Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ilan-gold Oct 25, 2025 •

edited

Loading

Zethson commented Oct 27, 2025 •

edited

Loading

flying-sheep commented Oct 31, 2025 •

edited

Loading

Zethson Nov 25, 2025 •

edited

Loading