Skip to content

fix(typing): make SVDynamics members read-only; unblock linter bumps - #299

Merged
thomaspinder merged 1 commit into
mainfrom
fix/ty-protocol-variance
Aug 7, 2026
Merged

fix(typing): make SVDynamics members read-only; unblock linter bumps#299
thomaspinder merged 1 commit into
mainfrom
fix/ty-protocol-variance

Conversation

@thomaspinder

Copy link
Copy Markdown
Owner

Unblocks #296 and fixes the one real bug it surfaced.

Why #296 fails

Dependabot's 16-package bump fails quality and all four tests-and-type-check legs. The runtime stack is fine — every failing leg prints 1111 passed, 1 skipped and then dies on uv run ty check. lowest-direct-deps, arviz-1-2-compat, wheel-install, build-docs and docs-linkcheck all pass, which is the tell: the floors are fine, a newly locked version isn't.

One package of the sixteen is responsible: ty 0.0.18 → 0.0.65, 47 releases of a pre-1.0 type checker, emitting 16 diagnostics. Reproduced against main with uvx --from ty==0.0.65 ty check.

Fifteen were tooling artifacts. One was a real bug.

The real bug: SVDynamics protocol variance

class SVDynamics(Protocol):
    name: str                    # mutable => invariant
    has_explicit_level: bool

A mutable protocol member is invariant, so RandomWalk.name: Literal["random_walk"] never actually satisfied name: str, and SV_DYNAMICS_REGISTRY: dict[str, type[SVDynamics]] was unassignable. ty 0.0.18 just didn't check it. Declaring both as read-only properties makes them covariant and the narrowing legal.

@runtime_checkable is unaffected — isinstance checks attribute presence, which the pydantic fields still provide. Confirmed: isinstance(RandomWalk(), SVDynamics) and isinstance(AR1(), SVDynamics) both still True, registry resolves, 208 SV/volatility tests pass.

Worth recording: Mapping[str, type[SVDynamics]] does not fix this. I measured it — still 16 diagnostics. Mapping's covariance is in the container, not the element check; each value must still be assignable to type[SVDynamics].

The 14 false positives

with pytest.raises(ValidationError):
    data.endog = np.zeros((100, 3))

The assignment is the assertion. ty ≥ 0.0.65 models pydantic frozen semantics and rejects statically the exact line whose runtime rejection is being asserted. invalid-assignment is now ignored for tests/** only — src/ keeps the rule, which is what surfaced the protocol bug above.

Four inline # ty: ignore[invalid-assignment] comments in test_sv_spec.py and test_volatility.py become redundant and are removed. They were earlier one-off patches for this same pattern, which is evidence the friction predates this bump.

The zip diagnostic

_check_finite used zip(frame.columns, finite, strict=True), which ty couldn't unpack (_T_co@zip is not iterable). Rewritten as a boolean mask over the index — shorter, faster, no suppression needed.

Dependabot grouping

ty, ruff and prek now group separately from runtime dependencies. Bundled under patterns: ["*"], one new lint rule blocks everything else in the same PR — #296 stalled numpy 2.4.6's fix for np.linalg.svd(..., hermitian=True) returning non-unitary vh (numpy#31347) behind a type-checker bump, and this library factorises covariance matrices.

Verification

Both halves of the pinned PyMC/ArviZ matrix, since the lock carries both families:

py3.11 / PyMC 5 / ArviZ 0 py3.12 / PyMC 6 / ArviZ 1
ty 0.0.18 (locked) clean clean
ty 0.0.65 (#296) clean clean
ty 0.0.67 (latest) clean clean
suite 1111 passed, 1 skipped 1112 passed

make check passes. The skip is test_arviz_compat.py:70, ArviZ-1-only, pre-existing.

Then, decisively: overlaid #296's uv.lock onto this branch and re-ran everything. ty 0.0.65 → All checks passed, prek run --all-files → all hooks pass, suite → 1111 passed. This branch is sufficient to unblock #296; that PR needs only a rebase afterwards.

Two things found but deliberately not fixed here

Unbounded ty floor. pyproject.toml has "ty>=0.0.14", so the lock is the only thing holding it at 0.0.18 — any uv lock regeneration reintroduces these 16 diagnostics, not just a dependabot PR. This branch makes that harmless rather than papering over it.

Two different ruffs. .pre-commit-config.yaml pins ruff-pre-commit at v0.14.14 (what prek and therefore CI enforce), while the lock's dev-dependency ruff is 0.15.2 → 0.16.1. They have already drifted: uv run ruff format --check under 0.16.1 wants to reformat 13 files that prek's 0.14.14 considers clean. Not a #296 blocker and not touched here, but dependabot.yml has no pre-commit ecosystem entry, so that rev is never bumped automatically. Worth its own PR.

#296 (dependabot, 16 packages) fails `quality` and all four
`tests-and-type-check` legs. The runtime stack is fine — every leg reports
1111 passed before dying on `uv run ty check`. One package is responsible:
ty 0.0.18 -> 0.0.65, which emits 16 diagnostics. Reproduced by running
`uvx --from ty==0.0.65 ty check` against main.

Fifteen were tooling artifacts. One was a real bug.

`SVDynamics` declared `name: str` and `has_explicit_level: bool` as mutable
protocol members. Mutable members are invariant, so `RandomWalk.name:
Literal["random_walk"]` never actually satisfied the protocol, and
`SV_DYNAMICS_REGISTRY: dict[str, type[SVDynamics]]` was unassignable.
Declaring them read-only properties makes the members covariant and the
narrowing legal. `@runtime_checkable` is unaffected: `isinstance` checks
attribute presence, which pydantic fields still provide.

Note `Mapping[str, type[SVDynamics]]` does *not* fix this — measured, still
16 diagnostics. Container covariance does not change the element check.

The remaining 14 were `invalid-assignment` on lines like

    with pytest.raises(ValidationError):
        data.endog = np.zeros((100, 3))

where the assignment *is* the assertion. ty >= 0.0.65 models pydantic
frozen semantics and rejects them statically. The rule is now ignored for
`tests/**` only; `src/` keeps it, which is what surfaced the protocol bug.
Four inline `# ty: ignore[invalid-assignment]` comments in test_sv_spec.py
and test_volatility.py are now redundant and removed — they were earlier
patches for this same pattern.

`_check_finite` used `zip(frame.columns, finite, strict=True)`, which ty
could not unpack. Rewritten as a boolean mask over the index: shorter, no
suppression needed.

Dependabot now groups ty/ruff/prek separately from runtime dependencies.
Bundled together, one new lint rule blocks everything else in the PR — #296
stalled numpy 2.4.6's fix for `np.linalg.svd(hermitian=True)` returning
non-unitary `vh`, which matters for a library factorising covariances.

Verified on both halves of the pinned matrix — ty 0.0.18, 0.0.65 and 0.0.67
all clean, `make check` passes, and the suite passes on py3.11/PyMC 5/ArviZ
0 (1111 passed) and py3.12/PyMC 6/ArviZ 1 (1112 passed).
@codecov-commenter

codecov-commenter commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.1%. Comparing base (51595aa) to head (14fd62a).

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #299   +/-   ##
=====================================
  Coverage   96.1%   96.1%           
=====================================
  Files         55      55           
  Lines       3547    3548    +1     
  Branches     441     441           
=====================================
+ Hits        3409    3410    +1     
  Misses       102     102           
  Partials      36      36           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thomaspinder
thomaspinder merged commit 83095a8 into main Aug 7, 2026
13 checks passed
@thomaspinder
thomaspinder deleted the fix/ty-protocol-variance branch August 7, 2026 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants