make cli tests pass

Author: d-v-b

src/zarr/codecs/blosc.py

@@ -232,7 +232,7 @@ def _from_json_v2(cls, data: CodecJSON) -> Self:
                 clevel=data["clevel"],
                 shuffle=BLOSC_SHUFFLE[data["shuffle"]],
                 blocksize=data["blocksize"],
-                typesize=data.get("typesize", 1),
+                typesize=data.get("typesize"),
             )
         msg = (
             "Invalid Zarr V2 JSON representation of the blosc codec. "

src/zarr/core/array.py

@@ -197,6 +197,44 @@ def parse_array_metadata(data: Any) -> ArrayMetadata:
     raise TypeError  # pragma: no cover
 
 
+def v2_to_v3_codecs(metadata: ArrayV2Metadata) -> tuple[Codec | NumcodecWrapper, ...]:
+    """
+    Convert the filters and compressor from Zarr v2 to a Zarr-V3-compatible sequence of codecs.
+    """
+    codecs: tuple[Codec | NumcodecWrapper, ...] = ()
+    if metadata.filters is not None:
+        codecs += metadata.filters
+    if metadata.compressor is not None:
+        codecs += (metadata.compressor,)
+    if not any(isinstance(codec, ArrayBytesCodec) for codec in codecs) and not isinstance(
+        metadata.dtype, HasObjectCodec
+    ):
+        # The role filled by the ArrayBytesCodec was implicit in zarr v2. So a valid zarr v2-style
+        # chain of filters + compressor might not contain a codec identifiable as an array-bytes codec.
+        # In such a case, we will insert a bytes codec that applies no endian transformation.
+        # We skip this insertion if the data type is an instance of HasObjectCodec, because
+        # in zarr v2 these data types required a special codec that functioned like an array bytes codec.
+
+        # find the last array-array codec, if any
+        abc_idx = 0
+        for idx, codec in enumerate(codecs):
+            if isinstance(codec, ArrayArrayCodec):
+                abc_idx = idx + 1
+        if isinstance(metadata.dtype, HasEndianness):
+            out_endianness = metadata.dtype.endianness
+        else:
+            out_endianness = None
+        codecs = codecs[:abc_idx] + (BytesCodec(endian=out_endianness),) + codecs[abc_idx:]
+    if metadata.order == "F":
+        # Zarr V2 supports declaring the order of an array in metadata. Using the zarr v3 codec
+        # framework, we express C or F ordered arrays by adding a transpose codec to the front
+        # of the list of codecs.
+        codecs = (TransposeCodec(order=tuple(reversed(range(metadata.ndim)))),) + codecs
+        # We ignore this type check failure because we don't want to change the type signature
+        # of the from_codecs method yet.
+    return codecs
+
+
 def create_codec_pipeline(metadata: ArrayMetadata, *, store: Store | None = None) -> CodecPipeline:
     if store is not None:
         try:
@@ -208,38 +246,8 @@ def create_codec_pipeline(metadata: ArrayMetadata, *, store: Store | None = None
 
     if isinstance(metadata, ArrayV3Metadata):
         return get_pipeline_class().from_codecs(metadata.codecs)
-    elif isinstance(metadata, ArrayV2Metadata):
-        _codecs: tuple[Codec | NumcodecWrapper, ...] = ()
-        if metadata.filters is not None:
-            _codecs += metadata.filters
-        if metadata.compressor is not None:
-            _codecs += (metadata.compressor,)
-        if not any(isinstance(codec, ArrayBytesCodec) for codec in _codecs) and not isinstance(
-            metadata.dtype, HasObjectCodec
-        ):
-            # The role filled by the ArrayBytesCodec was implicit in zarr v2. So a valid zarr v2-style
-            # chain of filters + compressor might not contain a codec identifiable as an array-bytes codec.
-            # In such a case, we will insert a bytes codec that applies no endian transformation.
-            # We skip this insertion if the data type is an instance of HasObjectCodec, because
-            # in zarr v2 these data types required a special codec that functioned like an array bytes codec.
-
-            # find the last array-array codec, if any
-            abc_idx = 0
-            for idx, codec in enumerate(_codecs):
-                if isinstance(codec, ArrayArrayCodec):
-                    abc_idx = idx + 1
-            if isinstance(metadata.dtype, HasEndianness):
-                out_endianness = metadata.dtype.endianness
-            else:
-                out_endianness = None
-            _codecs = _codecs[:abc_idx] + (BytesCodec(endian=out_endianness),) + _codecs[abc_idx:]
-        if metadata.order == "F":
-            # Zarr V2 supports declaring the order of an array in metadata. Using the zarr v3 codec
-            # framework, we express C or F ordered arrays by adding a transpose codec to the front
-            # of the list of codecs.
-            _codecs = (TransposeCodec(order=tuple(reversed(range(metadata.ndim)))),) + _codecs
-            # We ignore this type check failure because we don't want to change the type signature
-            # of the from_codecs method yet.
+    else:
+        _codecs = v2_to_v3_codecs(metadata)
         return get_pipeline_class().from_codecs(_codecs)  # type: ignore[arg-type]
     raise TypeError  # pragma: no cover
 

src/zarr/metadata/migrate_v3.py

@@ -1,20 +1,14 @@
 import asyncio
 import logging
-from typing import Literal, cast
-
-import numcodecs.abc
+from typing import TYPE_CHECKING, Literal, cast
 
 import zarr
 from zarr import Array, Group
-from zarr.abc.codec import ArrayArrayCodec, BytesBytesCodec, Codec
 from zarr.abc.store import Store
-from zarr.codecs.blosc import BloscCodec, BloscShuffle
-from zarr.codecs.bytes import BytesCodec
-from zarr.codecs.gzip import GzipCodec
-from zarr.codecs.transpose import TransposeCodec
-from zarr.codecs.zstd import ZstdCodec
+from zarr.core.array import v2_to_v3_codecs
 from zarr.core.buffer.core import default_buffer_prototype
 from zarr.core.chunk_key_encodings import V2ChunkKeyEncoding
+from zarr.core.codec_pipeline import codecs_from_list
 from zarr.core.common import (
     ZARR_JSON,
     ZARRAY_JSON,
@@ -23,15 +17,15 @@
     ZMETADATA_V2_JSON,
     ZarrFormat,
 )
-from zarr.core.dtype.common import HasEndianness
-from zarr.core.dtype.wrapper import TBaseDType, TBaseScalar, ZDType
 from zarr.core.group import GroupMetadata
 from zarr.core.metadata.v2 import ArrayV2Metadata
 from zarr.core.metadata.v3 import ArrayV3Metadata
 from zarr.core.sync import sync
-from zarr.registry import get_codec_class
 from zarr.storage import StorePath
 
+if TYPE_CHECKING:
+    from zarr.abc.codec import Codec
+
 _logger = logging.getLogger(__name__)
 
 
@@ -186,27 +180,10 @@ async def _metadata_exists(zarr_format: ZarrFormat, store_path: StorePath) -> bo
 def _convert_array_metadata(metadata_v2: ArrayV2Metadata) -> ArrayV3Metadata:
     chunk_key_encoding = V2ChunkKeyEncoding(separator=metadata_v2.dimension_separator)
 
-    codecs: list[Codec] = []
-
-    # array-array codecs
-    if metadata_v2.order == "F":
-        # F is equivalent to order: n-1, ... 1, 0
-        codecs.append(TransposeCodec(order=list(range(len(metadata_v2.shape) - 1, -1, -1))))
-
-    if metadata_v2.filters is not None:
-        codecs.extend(_convert_filters(metadata_v2.filters))
-
-    # array-bytes codecs
-    if not isinstance(metadata_v2.dtype, HasEndianness):
-        codecs.append(BytesCodec(endian=None))
-    else:
-        codecs.append(BytesCodec(endian=metadata_v2.dtype.endianness))
-
-    # bytes-bytes codecs
-    if metadata_v2.compressor is not None:
-        bytes_bytes_codec = _convert_compressor(metadata_v2.compressor, metadata_v2.dtype)
-        codecs.append(bytes_bytes_codec)
-
+    codecs: tuple[Codec, ...] = ()
+    # We first generate a sequence of V3 codecs, then we ensure that this sequence is valid
+    aa, ab, bb = codecs_from_list(v2_to_v3_codecs(metadata_v2))
+    codecs = (*aa, ab, *bb)
     return ArrayV3Metadata(
         shape=metadata_v2.shape,
         data_type=metadata_v2.dtype,
@@ -220,66 +197,6 @@ def _convert_array_metadata(metadata_v2: ArrayV2Metadata) -> ArrayV3Metadata:
     )
 
 
-def _convert_filters(filters: tuple[numcodecs.abc.Codec, ...]) -> list[ArrayArrayCodec]:
-    filters_codecs = [_find_numcodecs_zarr3(filter) for filter in filters]
-    for codec in filters_codecs:
-        if not isinstance(codec, ArrayArrayCodec):
-            raise TypeError(f"Filter {type(codec)} is not an ArrayArrayCodec")
-
-    return cast(list[ArrayArrayCodec], filters_codecs)
-
-
-def _convert_compressor(
-    compressor: numcodecs.abc.Codec, dtype: ZDType[TBaseDType, TBaseScalar]
-) -> BytesBytesCodec:
-    match compressor.codec_id:
-        case "blosc":
-            return BloscCodec(
-                typesize=dtype.to_native_dtype().itemsize,
-                cname=compressor.cname,
-                clevel=compressor.clevel,
-                shuffle=BloscShuffle.from_int(compressor.shuffle),
-                blocksize=compressor.blocksize,
-            )
-
-        case "zstd":
-            return ZstdCodec(
-                level=compressor.level,
-                checksum=compressor.checksum,
-            )
-
-        case "gzip":
-            return GzipCodec(level=compressor.level)
-
-        case _:
-            # If possible, find matching zarr.codecs.numcodecs codec
-            compressor_codec = _find_numcodecs_zarr3(compressor)
-
-            if not isinstance(compressor_codec, BytesBytesCodec):
-                raise TypeError(f"Compressor {type(compressor_codec)} is not a BytesBytesCodec")
-
-            return compressor_codec
-
-
-def _find_numcodecs_zarr3(numcodecs_codec: numcodecs.abc.Codec) -> Codec:
-    """Find matching zarr.codecs.numcodecs codec (if it exists)"""
-
-    numcodec_name = f"numcodecs.{numcodecs_codec.codec_id}"
-    numcodec_dict = {
-        "name": numcodec_name,
-        "configuration": numcodecs_codec.get_config(),
-    }
-
-    try:
-        codec_v3 = get_codec_class(numcodec_name)
-    except KeyError as exc:
-        raise ValueError(
-            f"Couldn't find corresponding zarr.codecs.numcodecs codec for {numcodecs_codec.codec_id}"
-        ) from exc
-
-    return codec_v3.from_dict(numcodec_dict)
-
-
 async def _save_v3_metadata(
     metadata_v3: ArrayV3Metadata | GroupMetadata, output_path: StorePath, dry_run: bool = False
 ) -> None:

tests/test_cli/test_migrate_v3.py

@@ -32,7 +32,9 @@
 
 runner = typer_testing.CliRunner()
 
-NUMCODECS_USER_WARNING = "Numcodecs codecs are not in the Zarr version 3 specification and may not be supported by other zarr implementations."
+UNSTABLE_SPEC_WARNING = (
+    "Data saved with this codec may not be supported by other Zarr implementations. "
+)
 
 
 def test_migrate_array(local_store: LocalStore) -> None:
@@ -316,8 +318,8 @@ def test_migrate_compressor(
     assert np.all(zarr_array[:] == 1)
 
 
-@pytest.mark.filterwarnings(f"ignore:{NUMCODECS_USER_WARNING}:UserWarning")
-def test_migrate_numcodecs_compressor(local_store: LocalStore) -> None:
+@pytest.mark.filterwarnings(f"ignore:.*{UNSTABLE_SPEC_WARNING}.*")
+def test_migrate_lzma_compressor(local_store: LocalStore) -> None:
     """Test migration of a numcodecs compressor without a zarr.codecs equivalent."""
 
     lzma_settings = {
@@ -360,7 +362,7 @@ def test_migrate_numcodecs_compressor(local_store: LocalStore) -> None:
     assert np.all(zarr_array[:] == 1)
 
 
-@pytest.mark.filterwarnings(f"ignore:{NUMCODECS_USER_WARNING}:UserWarning")
+@pytest.mark.filterwarnings(f"ignore:.*{UNSTABLE_SPEC_WARNING}.*")
 def test_migrate_filter(local_store: LocalStore) -> None:
     filter_v2 = numcodecs.Delta(dtype="<u2", astype="<u2")
     filter_v3 = Delta(dtype="<u2", astype="<u2")
@@ -504,11 +506,8 @@ def test_migrate_unknown_codec(local_store: LocalStore) -> None:
 
     result = runner.invoke(cli.app, ["migrate", "v3", str(local_store.root)])
     assert result.exit_code == 1
-    assert isinstance(result.exception, ValueError)
-    assert (
-        str(result.exception)
-        == "Couldn't find corresponding zarr.codecs.numcodecs codec for categorize"
-    )
+    assert isinstance(result.exception, KeyError)
+    assert str(result.exception) == "'categorize'"
 
 
 def test_migrate_incorrect_filter(local_store: LocalStore) -> None:
@@ -524,39 +523,29 @@ def test_migrate_incorrect_filter(local_store: LocalStore) -> None:
         fill_value=0,
     )
 
-    with pytest.warns(UserWarning, match=NUMCODECS_USER_WARNING):
-        result = runner.invoke(cli.app, ["migrate", "v3", str(local_store.root)])
+    result = runner.invoke(cli.app, ["migrate", "v3", str(local_store.root)])
 
-    assert result.exit_code == 1
-    assert isinstance(result.exception, TypeError)
-    assert (
-        str(result.exception)
-        == "Filter <class 'zarr.codecs.numcodecs._codecs.Zstd'> is not an ArrayArrayCodec"
-    )
+    assert result.exit_code == 0
 
 
-def test_migrate_incorrect_compressor(local_store: LocalStore) -> None:
-    """Attempting to convert a compressor (which is the wrong type of codec) should always fail"""
+def test_migrate_delta_compressor(local_store: LocalStore) -> None:
+    """Attempting to convert a numcodecs compressor should succeed"""
 
-    zarr.create_array(
-        store=local_store,
-        shape=(10, 10),
-        chunks=(10, 10),
-        dtype="uint16",
-        compressors=numcodecs.Delta(dtype="<u2", astype="<u2"),
-        zarr_format=2,
-        fill_value=0,
-    )
+    with pytest.warns(UserWarning, match=UNSTABLE_SPEC_WARNING):
+        zarr.create_array(
+            store=local_store,
+            shape=(10, 10),
+            chunks=(10, 10),
+            dtype="uint16",
+            compressors=numcodecs.Delta(dtype="<u2", astype="<u2"),
+            zarr_format=2,
+            fill_value=0,
+        )
 
-    with pytest.warns(UserWarning, match=NUMCODECS_USER_WARNING):
+    with pytest.warns(UserWarning, match=UNSTABLE_SPEC_WARNING):
         result = runner.invoke(cli.app, ["migrate", "v3", str(local_store.root)])
 
-    assert result.exit_code == 1
-    assert isinstance(result.exception, TypeError)
-    assert (
-        str(result.exception)
-        == "Compressor <class 'zarr.codecs.numcodecs._codecs.Delta'> is not a BytesBytesCodec"
-    )
+    assert result.exit_code == 0
 
 
 @pytest.mark.parametrize("zarr_format", [2, 3])