Repository navigation
chore: 🔧 enforce type checking with ty (fixes #16) - #37
Merged
Panadestein merged 5 commits intoOct 6, 2026
Merged
Conversation
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
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The unpinned development dependency can drift from the fixed CI hook version and produce inconsistent diagnostics.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
🤖 AI text below 🤖
Adds ty type checking to the project and CI lint flow, addressing issue #16.
Changes:
- Configures and hooks
tythroughprek. - Tightens NumPy dtype and array annotations.
- Resolves type-checking diagnostics and updates logging configuration.
| File | Description |
|---|---|
pyproject.toml |
Adds and configures ty. |
prek.toml |
Runs ty as a lint hook. |
uv.lock |
Locks the new dependency. |
src/src_method/apply.py |
Refines dtype annotations and suppressions. |
src/src_method/compress.py |
Refines dtype annotations and suppressions. |
src/src_method/utils/_backend.py |
Suppresses the optional CuPy import diagnostic. |
src/src_method/utils/linalg.py |
Adds typed NDArray parameters. |
src/src_method/utils/logging_config.py |
Updates logging-level handling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ty>=0.0.82 let uv.lock resolve newer than the pre-commit hook's pinned 0.0.82, so local checks and CI could silently diagnose differently. Pin exactly, matching the pattern already used for ruff.
Assisted-by: pi:claude-sonnet-5.5
Assisted-by: pi:claude-sonnet-5.5
Member
|
Thanks @pgrobasillobre ! |
Panadestein
added a commit
that referenced
this pull request
Oct 6, 2026
Finishes #28 on top of #37: ty now checks tests/ as well as src/, and a scheduled update-ty.yml bumps the ty pin in pyproject.toml and prek.toml together, opening the PR as a draft when the new version reports diagnostics. The one finding in tests/ was real: chi_out was annotated as int although NumPy integers are accepted on purpose. The public signatures now take int | np.integer, and validate_chi_out returns a plain int. Closes #28 Assisted-by: pi:claude-opus-5.5 Signed-off-by: Ramón L. Panadés-Barrueta <rpana92@gmail.com>
Panadestein
added a commit
that referenced
this pull request
Oct 7, 2026
Resolve the conflicts with stdlib logging (#52), input-precision sketches (#36), validation (#38), ty enforcement (#37, #55) and the Fumadocs site (#56): - the sweep draws its sketches with gaussian_sketch and logs its plan, pass times and stalls at DEBUG with %-style arguments; - the working dtype is promoted over every site, not just the first; - docs/large-problems.md moves to features/large-problems.mdx and the out-of-core testing notes to contributing/testing.mdx; - bench_large.py logs through the stdlib and gains --debug. Type lazily read sites: the entry points accept any array-like with shape, dtype, ndim and np.asarray support but were typed Sequence[NDArray]. Add the SiteLike protocol and the Site alias, use them from src/apply/compress down to the site source, and export SiteLike. Also fix the other ty findings: the Tier list in the planner, known_kind() for validated trains, padded_shape without a type: ignore, cupyx as an allowed unresolved import, and typed fakes in the tests. Assisted-by: pi:claude-opus-5.5
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.
🤖 AI text below 🤖
Adopts
tyas 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 theproject's existing
uv/ruffsetup.Silences the expected
unresolved-importon the optionalcupydependency, tightens
dtype: type→numpy.typing.DTypeLike(6x),and gives the bare
NDArrayinutils/linalg.pyits genericparameter. Also fixes a genuine, separate deprecation found along the
way in
logging_config.py.Note: the issue mentions
.pre-commit-config.yaml, but this repouses
prek.toml— thetyhook was added there instead. No changeneeded in
lint.yml, since it already runsprek.Known, out-of-scope finding:
ty checksurfaced 8 morediagnostics beyond the
cupyone mentioned in the issue —eta = [None] * n_sitesfilled via a loop, whichty's static analysiscan'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
assertbefore the final return) is agood follow-up, but touches working numerical code and felt out of
scope here.
Local-only note:
prek run --all-filesanduv sync --all-groups --all-extrasboth fail here (macOS/Apple Silicon) oncupy-cuda12x, which has no arm64 wheel. Confirmed this doesn'taffect CI, which runs on
ubuntu-latest.tyvsmypy: went withtyfor 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