chore: run mypy from developer environment#3972
Open
d-v-b wants to merge 14 commits into
Open
Conversation
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.
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.
Contributor
Author
|
another point: because the |
Codecov Report✅ All modified and coverable lines are covered by tests. 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
🚀 New features to boost your workflow:
|
chuckwondo
approved these changes
May 14, 2026
Contributor
chuckwondo
left a comment
chuckwondo
left a comment
There was a problem hiding this comment.
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 on lines
+1036
to
+1040
| # Legacy v2 behavior: an unspecified dtype defaults to float64. Resolve it here so the | ||
| # value forwarded to `_create` is a real dtype rather than `None`. | ||
| if dtype is None: | ||
| dtype = "float64" | ||
|
|
Contributor
There was a problem hiding this comment.
nitpick/personal pref: I'm not a fan of reassignment (not that I never use it, but I avoid it when it's easy/sensible). In this case, I suggest simply inlining this in the call to _create (i.e., simply change the argument there to dtype=dtype or "float64", and move the new comment above that line).
Contributor
Author
There was a problem hiding this comment.
yes! another "dislikes reassigning" person! I also dislike reassignment. happy to apply this suggestion here.
| """ | ||
| byte_order = endianness_to_numpy_str(self.endianness) | ||
| return self.dtype_cls().newbyteorder(byte_order) | ||
| return self.dtype_cls().newbyteorder(byte_order) # type: ignore[return-value] # numpy 2.x stub: newbyteorder widens to base dtype, runtime preserves the concrete subclass |
Contributor
There was a problem hiding this comment.
nitpick: can we split off the numpy stub comment and move it to the line above to avoid excessively long lines (to be applied to all similar comments in this PR)?
- 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>
Contributor
Author
|
thanks for the comments @chuckwondo, I integrated that feedback in the latest changes. let me know if anything else needs work! |
chuckwondo
reviewed
May 14, 2026
| with: | ||
| python-version: "3.12" | ||
| - name: Install Hatch | ||
| uses: pypa/hatch@257e27e51a6a5616ed08a39a408a21c35c9931bc |
Contributor
There was a problem hiding this comment.
Version number in a comment?
chuckwondo
reviewed
May 14, 2026
Contributor
chuckwondo
left a comment
chuckwondo
left a comment
There was a problem hiding this comment.
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.
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
2 participants
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.
On main, mypy gets its own python environment for pre-commit checks. This is problematic:
zarrto depend onzarr-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:mypyin 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.