Skip to content

chore: run mypy from developer environment#3972

Open
d-v-b wants to merge 15 commits into
zarr-developers:mainfrom
d-v-b:mypy-dev-env
Open

chore: run mypy from developer environment#3972
d-v-b wants to merge 15 commits into
zarr-developers:mainfrom
d-v-b:mypy-dev-env

Conversation

@d-v-b
Copy link
Copy Markdown
Contributor

@d-v-b d-v-b commented May 14, 2026

On main, mypy gets its own python environment for pre-commit checks. This is problematic:

  • it's a development burden because we need to update that environment every time we add a new optional dependency, or every time we raise the minimum version of a dependency.
  • It's host to some quirks: we have numpy pinned to a specific version simply because we do not have correct type annotations for the newer version of numpy, even through we support both versions.
  • As we want zarr to depend on zarr-metadata, we need mypy to type-check against the repo-local version of zarr-metadata, not the version on pypi. this is tedious to set up in the isolated pre-commit environment.

for these reasons, this claude-authored PR removes the mypy-specific environment setup in pre-commit.yml, and instead uses hatch run dev:mypy in pre-commit to simply invoke mypy in the pre-existing developer environment. To ensure consistency, this PR defines the developer environment more precisely by specifying a python version and committing a lockfile to ci.

because our dev environment does not pin numpy to a specific version, this change reveals type-checking failures on the newer version of numpy installed in the dev environment. so this PR addresses those failures, often by simply ignoring them. These ignore statements are temporary -- I will revisit this in a follow-up PR that actually fixes the underlying type-checking issue.

d-v-b added 9 commits May 14, 2026 11:24
Replace the pre-commit/mirrors-mypy hook (which maintained its own
duplicate dep list) with a `repo: local` hook that runs
`hatch run dev:mypy`. The dev hatch env's `dev` group (resolved via
uv.lock) becomes the single source of truth for mypy's dependency set.

This also unpins numpy from the type-check environment (it was
hard-pinned to `numpy==2.1` in the old hook); type fixes that follow
keep mypy clean against current numpy stubs:

  - relax NDArrayLike.reshape/all signatures so np.ndarray
    structurally satisfies the protocol
  - widen AsyncGroup.require_array's `dtype` to include None
  - add narrowly-scoped `# type: ignore` comments with explanatory
    notes where numpy 2.x stubs are too strict against runtime-valid
    calls (datetime64 unit f-strings, 'generic' unit sentinel,
    newbyteorder subclass identity, ZDTypeLike None handling)
  - drop stale `# type: ignore` comments that are no longer needed
`create()` accepts `dtype=None` (legacy v2 behavior: an unspecified
dtype defaults to float64). Previously this `None` was forwarded
untyped into `_create`, which doesn't accept `None` — it only worked
because `parse_dtype(None)` -> `np.dtype(None)` happens to resolve to
float64. That required a `cast()` to silence mypy.

Resolve `None` to `"float64"` explicitly in `create()` before
forwarding, so the value passed to `_create` is a real dtype and the
cast is no longer needed. No behavior change.
The initial fix for numpy-stub conformance widened the NDArrayLike
protocol's `reshape` and `all` to `(*args: Any, **kwargs: Any) -> Any`,
which erased type information for every consumer of the protocol.

Replace with precise signatures that np.ndarray still satisfies
structurally:

  - `reshape(shape: tuple[int, ...], /, *, order=..., copy=...)
    -> NDArrayLike` — the `Literal[-1]` form was the only thing
    blocking a precise signature (it straddles numpy's arity-split
    overloads); it is unused on protocol-typed values, so drop it.
    `NDBuffer.reshape` keeps its public `-1` support by normalizing
    `-1` to `(-1,)` before forwarding.
  - `all(self) -> np.bool_` — the sole caller wraps the result in
    `bool(...)`, and no-arg is all we use.
uv.lock was removed in zarr-developers#3962 as unused. The mypy-via-hatch change in
this branch makes it load-bearing again: it is the single source of
truth that keeps the `dev` hatch environment (and therefore mypy's
results) consistent across developer machines and CI. Restore it,
regenerated against the current pyproject.toml.
d-v-b added 2 commits May 14, 2026 11:33
The mypy hook is now `language: system` and shells out to
`hatch run dev:mypy`, which needs the project's hatch dev environment.
pre-commit.ci's hosted runners don't have it, so the hook can only
fail there. Add it to `ci.skip`; mypy is still covered by the Lint
GitHub Actions workflow (which installs hatch) and by local prek runs.
@d-v-b
Copy link
Copy Markdown
Contributor Author

d-v-b commented May 14, 2026

another point: because the pre-commit.ci CI job doesn't clone our repo locally, it can't use the new mypy setup. so mypy is being skipped in pre-commit ci (it happens in the lint workflow)

@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.29%. Comparing base (e815b51) to head (3f702cb).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3972   +/-   ##
=======================================
  Coverage   93.29%   93.29%           
=======================================
  Files          87       87           
  Lines       11752    11753    +1     
=======================================
+ Hits        10964    10965    +1     
  Misses        788      788           
Files with missing lines Coverage Δ
src/zarr/_compat.py 37.83% <100.00%> (ø)
src/zarr/api/asynchronous.py 94.05% <ø> (ø)
src/zarr/codecs/cast_value.py 98.60% <ø> (ø)
src/zarr/core/buffer/core.py 82.87% <100.00%> (+0.11%) ⬆️
src/zarr/core/dtype/__init__.py 100.00% <100.00%> (ø)
src/zarr/core/dtype/npy/int.py 99.37% <100.00%> (ø)
src/zarr/core/dtype/npy/string.py 92.30% <100.00%> (ø)
src/zarr/core/dtype/npy/time.py 98.85% <100.00%> (ø)
src/zarr/core/group.py 94.98% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@chuckwondo chuckwondo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic @d-v-b!

I like that although you had to add a few "ignore" comments, you were also able to remove some existing ones, particularly since some of those existing ignores were head-scratchers.

I made a couple of nitpicky suggestions, but non-blocking, so feel free to ignore. Approving.

Comment thread src/zarr/api/asynchronous.py Outdated
Comment thread src/zarr/core/dtype/npy/int.py Outdated
d-v-b and others added 3 commits May 14, 2026 13:03
- Inline the float64 dtype default into the `_create` call instead of
  reassigning the `dtype` variable.
- Move the numpy 2.x stub explanation onto its own line above the code
  so `# type: ignore` comment lines stay short.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@d-v-b
Copy link
Copy Markdown
Contributor Author

d-v-b commented May 14, 2026

thanks for the comments @chuckwondo, I integrated that feedback in the latest changes. let me know if anything else needs work!

Comment thread .github/workflows/lint.yml Outdated
with:
python-version: "3.12"
- name: Install Hatch
uses: pypa/hatch@257e27e51a6a5616ed08a39a408a21c35c9931bc
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version number in a comment?

Copy link
Copy Markdown
Contributor

@chuckwondo chuckwondo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I run hatch run dev:mypy, I get errors (same when running hatch run dev:pre-commit run -a, so at least we have consistency now):

tests/test_examples.py: note: In function "set_dep":
tests/test_examples.py:42: error: Argument 1 to "tuple" has incompatible type "Item | Container"; expected "Iterable[Any]"  [arg-type]
        for idx, dep in enumerate(tuple(config["dependencies"])):
                                        ^~~~~~~~~~~~~~~~~~~~~~
tests/test_examples.py:44: error: Unsupported target for indexed assignment ("Item | Container")  [index]
                config["dependencies"][idx] = dependency
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/test_examples.py:44: note: See https://mypy.rtfd.io/en/stable/_refs.html#code-index for more info
tests/test_examples.py:44: error: Invalid index type "int" for "Container"; expected type "Key | str"  [index]
                config["dependencies"][idx] = dependency
                                       ^~~
tests/test_array.py: note: In function "test_from_array":
tests/test_array.py:1795: error: Statement is unreachable  [unreachable]
        assert result.chunks == new_chunks
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/test_array.py:1795: note: See https://mypy.rtfd.io/en/stable/_refs.html#code-unreachable for more info

Oddly, VSCode does not flag these, even after restarting.

`hatch run dev:mypy` does not consume `uv.lock` — hatch has no lockfile
support and re-resolves the `dev` dependency group from scratch each time
it builds the environment. This defeated the PR's goal of a reproducible
type-checking environment: contributors with stale or differently-resolved
hatch `dev` envs saw different mypy results (e.g. errors from an older
`tomlkit` whose `TOMLDocument.__getitem__` was typed `Item | Container`
rather than `Any`).

Switch the mypy pre-commit hook and the Lint workflow to `uv run --frozen
mypy`. `uv` does sync from `uv.lock`, so the committed lockfile becomes the
real single source of truth for mypy's dependency set, identical for every
contributor and for CI.

- .pre-commit-config.yaml: hook entry `hatch run dev:mypy` -> `uv run --frozen mypy`
- .github/workflows/lint.yml: install `uv` instead of `hatch`
- pyproject.toml / changes: update wording to match

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@d-v-b
Copy link
Copy Markdown
Contributor Author

d-v-b commented May 14, 2026

@chuckwondo the issue was dependency skew -- hatch doesn't use lockfiles (yet), so there was nothing keeping our two dev envs aligned. the recent changes use uv, which should keep us in sync via the lockfile.

@d-v-b d-v-b requested a review from ilan-gold May 14, 2026 16:36
@chuckwondo
Copy link
Copy Markdown
Contributor

@chuckwondo the issue was dependency skew -- hatch doesn't use lockfiles (yet), so there was nothing keeping our two dev envs aligned. the recent changes use uv, which should keep us in sync via the lockfile.

Great! I get success now via pre-commit.

@d-v-b
Copy link
Copy Markdown
Contributor Author

d-v-b commented May 14, 2026

it looks like lockfile support is coming to hatch soon: pypa/hatch#2176. until then, using uv + uv.lock is our best bet

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