Skip to content

mypy fixes for NumPy 2.1.0 #2139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ repos:
hooks:
- id: check-yaml
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.11.1
rev: v1.11.2
hooks:
- id: mypy
files: src
Expand Down
8 changes: 5 additions & 3 deletions src/zarr/codecs/crc32c_.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from __future__ import annotations

from dataclasses import dataclass
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, cast

import numpy as np
import typing_extensions
from crc32c import crc32c

from zarr.abc.codec import BytesBytesCodec
Expand Down Expand Up @@ -37,7 +38,8 @@ async def _decode_single(
crc32_bytes = data[-4:]
inner_bytes = data[:-4]

computed_checksum = np.uint32(crc32c(inner_bytes)).tobytes()
# Need to do a manual cast until https://github.com/numpy/numpy/issues/26783 is resolved
computed_checksum = np.uint32(crc32c(cast(typing_extensions.Buffer, inner_bytes))).tobytes()
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't inner_bytes a numpy array, not a buffer here? So it needs to be converted to a buffer, not just have it's type cast? Maybe I'm misunderstanding something here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, it is an ndarray. At least as far as my copy of mypy is concerned, that's not sufficient for this type check to pass. This might have changed in newer versions of python though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are ndarrays also Buffers in reality then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the definition of Buffer: https://github.com/python/typing_extensions/blob/d9509f902c20e3d51c62b8abe522809b4760c0ff/src/typing_extensions.py#L3316-L3342

In python 3.12, a new ABC was added, to cover the addition of a Python API to the buffer protocol: https://peps.python.org/pep-0688/

For 3.11 and earlier, ndarrays are compatible with Buffer (they implement the buffer protocol) but apparently mypy doesn't know that.

stored_checksum = bytes(crc32_bytes)
if computed_checksum != stored_checksum:
raise ValueError(
Expand All @@ -52,7 +54,7 @@ async def _encode_single(
) -> Buffer | None:
data = chunk_bytes.as_numpy_array()
# Calculate the checksum and "cast" it to a numpy array
checksum = np.array([crc32c(data)], dtype=np.uint32)
checksum = np.array([crc32c(cast(typing_extensions.Buffer, data))], dtype=np.uint32)
# Append the checksum (as bytes) to the data
return chunk_spec.prototype.buffer.from_array_like(np.append(data, checksum.view("b")))

Expand Down
6 changes: 4 additions & 2 deletions src/zarr/codecs/sharding.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from enum import Enum
from functools import lru_cache
from operator import itemgetter
from typing import TYPE_CHECKING, Any, NamedTuple
from typing import TYPE_CHECKING, Any, NamedTuple, cast

import numpy as np
import numpy.typing as npt
Expand Down Expand Up @@ -101,7 +101,9 @@ class _ShardIndex(NamedTuple):

@property
def chunks_per_shard(self) -> ChunkCoords:
return self.offsets_and_lengths.shape[0:-1]
result = tuple(self.offsets_and_lengths.shape[0:-1])
# The cast is required until https://github.com/numpy/numpy/pull/27211 is merged
return cast(ChunkCoords, result)

def _localize_chunk(self, chunk_coords: ChunkCoords) -> ChunkCoords:
return tuple(
Expand Down
16 changes: 14 additions & 2 deletions src/zarr/core/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -602,9 +602,21 @@ def parse_fill_value_v3(fill_value: Any, dtype: FLOAT_DTYPE) -> FLOAT: ...
def parse_fill_value_v3(fill_value: Any, dtype: COMPLEX_DTYPE) -> COMPLEX: ...


@overload
def parse_fill_value_v3(fill_value: Any, dtype: np.dtype[Any]) -> Any:
# This dtype[Any] is unfortunately necessary right now.
# See https://github.com/zarr-developers/zarr-python/issues/2131#issuecomment-2318010899
# for more details, but `dtype` here (which comes from `parse_dtype`)
# is np.dtype[Any].
#
# If you want the specialized types rather than Any, you need to use `np.dtypes.<dtype>`
# rather than np.dtypes(<type>)
...


def parse_fill_value_v3(
fill_value: Any, dtype: BOOL_DTYPE | INTEGER_DTYPE | FLOAT_DTYPE | COMPLEX_DTYPE
) -> BOOL | INTEGER | FLOAT | COMPLEX:
fill_value: Any, dtype: BOOL_DTYPE | INTEGER_DTYPE | FLOAT_DTYPE | COMPLEX_DTYPE | np.dtype[Any]
) -> BOOL | INTEGER | FLOAT | COMPLEX | Any:
"""
Parse `fill_value`, a potential fill value, into an instance of `dtype`, a data type.
If `fill_value` is `None`, then this function will return the result of casting the value 0
Expand Down