chore: migrate from mypy to ty#117
Conversation
| entry: uvx ty check | ||
| language: system | ||
| types: | ||
| - python |
There was a problem hiding this comment.
In my local experiments, this works just fine without pass_filenames: false. So, in the name of locality, I'd leave that out.
| hooks: | ||
| - id: ty-check | ||
| name: ty-check | ||
| entry: uvx ty check |
There was a problem hiding this comment.
@hofbi, you told me that you solved this not via uvx, but via additional_dependencies and calling ty check directly. My LLM told me that that's not great because ty will then run in prek's venv and not see the project's venv, so you'd need to call ty check --python .venv. However, folder .venv is created by uv, so you might as well directly use uvx ty check. That's what I'm opting for here. Am I missing something?
There was a problem hiding this comment.
There was a problem hiding this comment.
Doesn't seem to work, see my response there.
Generally, this relates to astral-sh/ty#269, and we're discussing the two options also proposed in that issue:
The second proposal has more likes in that thread, but not sure if that's an indicator of quality. Either implementation will make using ty-check in pre-commit.ci impossible: either because of lack of uv, or lack of the .venv folder. Are you ok with skipping ty in CI? Or should we drop pre-commit.ci in favor of a GitHub action?
There was a problem hiding this comment.
We can skip in pre-commit.ci if it causes issues. I already created
dev-tools/.github/workflows/check.yaml
Lines 16 to 24 in 37d3c3d
When calling ty directly (ty being installed via additional deps), we can set
- https://docs.astral.sh/ty/reference/configuration/#error-on-warning (to error on warnings)
- For project discovery call with
--project .(https://docs.astral.sh/ty/reference/cli/#ty-check--project) or set environment root (https://docs.astral.sh/ty/modules/#first-party-modules)
There was a problem hiding this comment.
Ok, then we can skip ty-check in pre-commit ci. Unfortunately, we'll lose the nice autofixes from ty like that. I guess you don't want to reimplement the pre-commit.ci autofix pushes via something like I'm doing in a private gitlab project?
if [ "${CI_PIPELINE_SOURCE}" = "merge_request_event" ] && ! git diff --quiet; then
git config user.name "${GIT_COMMITTER_NAME:-GitLab CI}"
git config user.email "${GIT_COMMITTER_EMAIL:-ci@example.com}"
git add --update
git commit -m "chore: apply pre-commit autofixes"
git push "https://oauth2:${CI_PUSH_TOKEN}@${CI_SERVER_HOST}/${CI_PROJECT_PATH}.git" "HEAD:${CI_COMMIT_REF_NAME}"
fierror-on-warnings is already enabled via pyproject.toml.
I just tried --project . as arg for ty check: that fixes local imports, but gives the same errors reported here for 3rd-party imports.
The proposal using additional_dependencies plus --python=.venv works just fine.
What shall it be?
There was a problem hiding this comment.
I like the --python=.venv approach. Let's do that.
There was a problem hiding this comment.
Done, now you only need to skip ty-check in pre-commit.ci.
There was a problem hiding this comment.
You can do this with
dev-tools/.pre-commit-config.yaml
Lines 8 to 9 in 37d3c3d
There was a problem hiding this comment.
Oops, pre-commit.ci noob here. Done!
| [tool.mypy] | ||
| check_untyped_defs = true | ||
| disallow_incomplete_defs = true | ||
| disallow_untyped_calls = true | ||
| disallow_untyped_defs = true | ||
| ignore_missing_imports = true | ||
| no_implicit_optional = true | ||
| show_column_numbers = true | ||
| show_error_codes = true | ||
| show_error_context = true | ||
| strict_equality = true | ||
| strict_optional = true | ||
| warn_no_return = true | ||
| warn_redundant_casts = true | ||
| warn_return_any = true | ||
| warn_unused_configs = true | ||
| warn_unused_ignores = true |
There was a problem hiding this comment.
These are all on in ty per default
| - id: ty-check | ||
| name: ty-check | ||
| entry: uvx ty check | ||
| language: system |
There was a problem hiding this comment.
| language: system | |
| language: python |
There was a problem hiding this comment.
Doesn't work, unfortunately. I'm getting many errors like
error[unresolved-import]: Cannot resolve imported module `pre_commit.constants`
--> dev_tools/check_useless_exclude_paths_hooks.py:13:6
|
13 | from pre_commit.constants import CONFIG_FILE
| ^^^^^^^^^^^^^^^^^^^^
|
info: Searched in the following paths during module resolution:
info: 1. /home/chris/dev-tools (first-party code)
info: 2. vendored://stdlib (stdlib typeshed stubs vendored by ty)
info: 3. /home/chris/.cache/prek/hooks/python-pbR08CWFH1YCCY8xTzLd/lib/python3.13/site-packages (site-packages)
info: 4. /home/chris/.cache/prek/hooks/python-pbR08CWFH1YCCY8xTzLd/lib64/python3.13/site-packages (site-packages)
info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment
With language: python, prek creates an isolated venv for ty, which hides other dependencies.
cbachhuber
force-pushed
the
feat/migrate-from-mypy-to-ty
branch
from
9673661 to
f169215
Compare
| "pytest-cov", | ||
| "pyfakefs" | ||
| "pyfakefs", | ||
| "types-regex" |
There was a problem hiding this comment.
Do you think this would work when adding to the additional dependencies as we do for ty itself? Iguess not since we point ty to our venv, but just wanted to double check.
There was a problem hiding this comment.
Doesn't work. Probably because we point ty to uv's .venv, so it doesn't see modules added to prek's venv via additional_dependencies.
2 participants
Closes #115