-
Notifications
You must be signed in to change notification settings - Fork 181
Add type-checking #2174
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: main
Are you sure you want to change the base?
Add type-checking #2174
Changes from all commits
c2c517d
d01dbda
5558e9e
da08894
94bff07
f17d1b4
bf2d18a
e692b90
91ed109
40c895f
b067c8e
aa1c237
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| name: mypy | ||
|
|
||
| on: | ||
| push: | ||
| pull_request: | ||
|
|
||
| jobs: | ||
| mypy: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v5 | ||
| - name: Extract max Python version from classifiers | ||
| run: | | ||
| classifiers=$(yq .project.classifiers pyproject.toml -oy | grep --only-matching --perl-regexp '(?<=Python :: )(\d\.\d+)') | ||
| max_version=$(echo "$classifiers" | sort -V | tail -1) | ||
| echo "max_python_version=$max_version" >> $GITHUB_ENV | ||
| - uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: ${{ env.max_python_version }} | ||
| - name: Install mypy | ||
| run: | | ||
| pip install 'mypy @ git+https://github.com/python/mypy.git' | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even if we were to use this workflow, we shouldn't install from Github. We can just use a release. |
||
| - name: Run mypy | ||
| run: | | ||
| mypy --show-traceback src | ||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||
| ci: | ||||
| autoupdate_commit_msg: "ci: pre-commit autoupdate" | ||||
| skip: [mypy] # too big | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Zethson did you test that it works?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No I did not but I have never had to use it anywhere. But you're probably right then |
||||
|
|
||||
| repos: | ||||
| - repo: https://github.com/astral-sh/ruff-pre-commit | ||||
|
|
@@ -39,3 +40,25 @@ repos: | |||
| - id: codespell | ||||
| additional_dependencies: | ||||
| - tomli | ||||
| # See https://github.com/scverse/anndata/pull/2174 | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can also exclude them like this: https://github.com/laminlabs/lamindb/blob/main/.pre-commit-config.yaml#L71 |
||||
| # Running `pre-commit run -a` gives the following error: | ||||
| # tests/conftest.py: error: Duplicate module named "conftest" (also at "./tests/lazy/conftest.py") | ||||
| # tests/conftest.py: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#mapping-file-paths-to-modules for more info | ||||
| # This seems to be the same failure we get on the prec-commit.ci | ||||
| - repo: https://github.com/pre-commit/mirrors-mypy | ||||
| rev: v1.18.2 | ||||
| hooks: | ||||
| - id: mypy | ||||
| args: [--config-file=pyproject.toml, --tb, .] | ||||
| pass_filenames: false | ||||
| additional_dependencies: | ||||
| - array-api-compat | ||||
| - h5py | ||||
| - legacy-api-wrap | ||||
| - natsort | ||||
| - numpy | ||||
| - packaging | ||||
| - pandas | ||||
| - scipy | ||||
| - types-docutils | ||||
| - zarr | ||||
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.
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.