Skip to content

chore: 🔧 enforce type checking with ty (fixes #16) - #37

Open
pgrobasillobre wants to merge 1 commit into
Algorithmiq:mainfrom
pgrobasillobre:fix/16-type-checking-ci
Open

pgrobasillobre wants to merge 1 commit into
Algorithmiq:mainfrom
pgrobasillobre:fix/16-type-checking-ci

Conversation

@pgrobasillobre

Copy link
Copy Markdown

🤖 AI text below 🤖

Adopts ty as the type checker, replacing the dead [tool.pyright]
block (never invoked anywhere) with a working [tool.ty] config,
wired into prek.toml. Keeps the toolchain consistent with the
project's existing uv/ruff setup.

Silences the expected unresolved-import on the optional cupy
dependency, tightens dtype: typenumpy.typing.DTypeLike (6x),
and gives the bare NDArray in utils/linalg.py its generic
parameter. Also fixes a genuine, separate deprecation found along the
way in logging_config.py.

Note: the issue mentions .pre-commit-config.yaml, but this repo
uses prek.toml — the ty hook was added there instead. No change
needed in lint.yml, since it already runs prek.

Known, out-of-scope finding: ty check surfaced 8 more
diagnostics beyond the cupy one mentioned in the issue — eta = [None] * n_sites filled via a loop, which ty's static analysis
can't fully verify across a dynamic loop range, though the code and
tests are correct. Suppressed with targeted ignores; a proper fix
(local-variable capture + an assert before the final return) is a
good follow-up, but touches working numerical code and felt out of
scope here.

Local-only note: prek run --all-files and uv sync --all-groups --all-extras both fail here (macOS/Apple Silicon) on
cupy-cuda12x, which has no arm64 wheel. Confirmed this doesn't
affect CI, which runs on ubuntu-latest.

ty vs mypy: went with ty for toolchain consistency,
understanding it's pre-1.0 (per the issue's own caveat). Happy to
switch if preferred.

Testing: uv run ty check src/, uv run ruff check src/ tests/,
uv run ruff format --check src/ tests/, uv run pytest -m "not slow",
and uv run pytest (full suite) all pass locally.

AI/LLM disclosure

  • I did not use LLM tooling, or used it only privately for ideation
  • Used to help write this PR description: Claude (claude.ai, Sonnet 5)
  • Used to generate or modify code: Claude (claude.ai, Sonnet 5) — suggested specific code changes throughout; I applied every edit myself and verified each one (ty check, tests) before committing

Adopts ty as the type checker, replacing the dead [tool.pyright] block
(never invoked anywhere) with a working [tool.ty] config, wired into
prek.toml. ty keeps the toolchain consistent with the project's
existing uv/ruff setup.

Silences the expected unresolved-import diagnostic on the optional
cupy dependency, and tightens loose annotations: dtype: type becomes
numpy.typing.DTypeLike, and the bare NDArray in utils/linalg.py now
carries its generic parameter. Also fixes a genuine, separate
deprecated-API warning found along the way in logging_config.py.

Full details, including a known out-of-scope finding and a local-only
environment note, in the PR description.

Assisted-by: claude.ai:claude-sonnet-5
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.

1 participant