chore: replace mypy with ty for type checking - #784
Open
joshmarkovic wants to merge 2 commits into
Open
Conversation
Completes the Astral toolchain consolidation started in dbt-msft#707 and dbt-msft#711: Ruff for lint and format, ty for types. Scope and rules move into [tool.ty] in pyproject.toml, and ty is pinned there as the single source of the version, read by the pre-commit hook, `make ty` and CI alike. Coverage is unchanged: the adapter package only, with unresolvable `dbt.*` imports ignored, which is what mypy's --ignore-missing-imports did. `dbt` is split across distributions, so no static checker can join dbt-adapters' `dbt/adapters/` in site-packages with this repo's `dbt/adapters/sqlserver`. ty cannot run on pre-commit.ci, which blocks the network access ty needs to resolve dependencies, so the hook is skipped there and a Type check workflow runs it in GitHub Actions instead. Without that job the swap would drop type checking from CI entirely. Source changes needed to reach a clean baseline: - Annotate `Column: Type[SQLServerColumn]`, which __init__ overwrites with the SQLServerColumnNative subclass. - Suppress `missing-typed-dict-key` and `invalid-return-type` on the behavior flags. dbt-common declares BehaviorFlag's optional keys with a NotRequired that falls back to Optional under a try/except ImportError shim, so every key reads as required. - Suppress `unknown-argument` on the two SQLServerIndexConfigChange constructions, whose `action` field comes from the unresolvable RelationConfigChange base. - Drop the `# type: ignore` on SQLServerRelation.type, which suppressed nothing and ty reports as unused. Closes dbt-msft#712
joshmarkovic
marked this pull request as ready for review
July 28, 2026 17:55
joshmarkovic
marked this pull request as draft
July 28, 2026 18:52
joshmarkovic
marked this pull request as ready for review
July 30, 2026 13:38
Collaborator
|
@joshmarkovic taking into account the failure requieres changes, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #712.
What
Replaces mypy with
ty, completing the Astral toolchain consolidation from #707 (lint) and #711 (format). Scope and rules live in[tool.ty]inpyproject.toml;ty==0.0.64in the dev group is the single source of the version, read by the pre-commit hook,make tyand CI alike..pre-commit-config.yaml: twomirrors-mypyhooks become one local hook runninguv run --frozen ty check..github/workflows/type-check.yml: new job running the same command.Makefile:make mypybecomesmake ty;lintandtestupdated..gitignore:.mypy_cache/dropped, ty writes no cache.Coverage is unchanged
The adapter package only, with unresolvable
dbt.*imports ignored, which is what mypy's--ignore-missing-importsdid.dbtis split across distributions: dbt-core and dbt-adapters owndbt/anddbt/adapters/in site-packages, this repo contributesdbt/adapters/sqlserver, and the editable install joins them at runtime via an import hook that no static checker can follow.Why ty runs in Actions and not on pre-commit.ci
pre-commit.ci blocks network access while hooks run, and ty needs it to resolve this project's dependencies. Astral's own recommendation is
ci: skipplus a separate CI invocation. pre-commit.ci is this repo's only lint runner today, so without the new job the swap would drop type checking from CI entirely. If a general lint workflow lands later, this job folds into it.A local hook is used rather than
astral-sh/ty-pre-commitso the version is declared once: the upstream hook hardcodes--ty-versionin its entry, which would mean arevpin and a dev-group pin drifting apart, the same dual-pin mypy had. Version bumps now arrive through the existing dependabot pip group as apyproject.toml+uv.lockchange. The hook needsuvonPATH, already a requirement viamake dev.Source changes
Fourteen touches to reach a clean baseline:
Column: Type[SQLServerColumn], which__init__overwrites with theSQLServerColumnNativesubclass. The only real fix.BehaviorFlag's optional keys with aNotRequiredthat falls back toOptionalunder atry/except ImportErrorshim, sosourceanddocs_urlread as required. Reproduced in isolation; the suppressions go stale (and ty says so) once that shim is dropped.unknown-argumentsuppressions onSQLServerIndexConfigChange, whoseactionfield comes from the unresolvableRelationConfigChangebase.# type: ignoreonSQLServerRelation.type, which suppressed nothing and ty reports as unused.Inline suppressions rather than disabling
invalid-return-type,unknown-argumentandmissing-typed-dict-keyproject-wide, so those rules keep working everywhere else.Two corrections to the issue
pathspecoverride was removed in the dbt-core 1.12 upgrade (dd61d69), and pathspec now resolves to 1.0.4 on its own.dbt/adapters: mypy 2.1.0 at 2.7s cold and 0.19s warm cache, ty at 0.12s. Real but not the 28x in the issue. The durable wins are one toolchain and config inpyproject.toml.Worth flagging: ty is still 0.0.x with no stability guarantees, hence the exact pin.
Testing
pre-commit run -apasses, including ty.uv run --frozen ty check, the CI command, is clean.-> int: return "not an int"and confirmed the hook fails on it, so the config is not a no-op.--frozenleavesuv.lockbyte-identical, so a commit cannot silently relock.Follow-up
With an extra search path pointing at site-packages, ty does resolve
dbt.*and reports 9 findings mypy never saw: anAdapterPluginprotocol mismatch,from_dictincompatible acrossRelationConfigBaseandDataClassDictMixin,mp_context=NoneagainstSpawnContext, acan_expand_tooverride incompatibility, and an unresolved attribute onself.connections. That search path is not committable, it hardcodes a Python version and breaks in CI, but the findings are real and deserve their own issue.