Fix order handling

Author: dstansby

changes/xxx1.bugfix.rst

@@ -0,0 +1 @@
+Fixed the error message when passing both ``config`` and ``write_emtpy_chunks`` arguments to reflect the current behaviour (``write_empty_chunks`` takes precedence).

changes/xxx2.bugfix.rst

@@ -0,0 +1 @@
+Creating a Zarr format 2 array with the ``order`` keyword argument no longer raises a warning.

changes/xxx3.bugfix.rst

@@ -0,0 +1 @@
+Creating a Zarr format 3 array with the ``order`` argument now conistently ignores this argument and raises a warning.

changes/xxx4.bugfix.rst

@@ -0,0 +1,2 @@
+When using ``from_array`` to copy a Zarr format 2 array to a Zarr format 3 array, if the memory order of the input array is ``"F"`` a warning is raised and the order ignored.
+This is because Zarr format 3 arrays are always stored in "C" order.

changes/xxxx.bugfix.rst

@@ -0,0 +1 @@
+The ``config`` argument to `zarr.create` (and functions that create arrays) is now used - previously it had no effect.

src/zarr/api/asynchronous.py

@@ -18,7 +18,7 @@
     from_array,
     get_array_metadata,
 )
-from zarr.core.array_spec import ArrayConfig, ArrayConfigLike, ArrayConfigParams
+from zarr.core.array_spec import ArrayConfigLike, parse_array_config
 from zarr.core.buffer import NDArrayLike
 from zarr.core.common import (
     JSON,
@@ -28,7 +28,6 @@
     MemoryOrder,
     ZarrFormat,
     _default_zarr_format,
-    _warn_order_kwarg,
     _warn_write_empty_chunks_kwarg,
     parse_dtype,
 )
@@ -1028,8 +1027,6 @@ async def create(
         warnings.warn("object_codec is not yet implemented", RuntimeWarning, stacklevel=2)
     if read_only is not None:
         warnings.warn("read_only is not yet implemented", RuntimeWarning, stacklevel=2)
-    if order is not None:
-        _warn_order_kwarg()
     if write_empty_chunks is not None:
         _warn_write_empty_chunks_kwarg()
 
@@ -1041,26 +1038,17 @@ async def create(
         mode = "a"
     store_path = await make_store_path(store, path=path, mode=mode, storage_options=storage_options)
 
-    config_dict: ArrayConfigParams = {}
+    config_parsed = parse_array_config(config)
 
     if write_empty_chunks is not None:
         if config is not None:
             msg = (
                 "Both write_empty_chunks and config keyword arguments are set. "
-                "This is redundant. When both are set, write_empty_chunks will be ignored and "
-                "config will be used."
+                "This is redundant. When both are set, write_empty_chunks will be used instead "
+                "of the value in config."
             )
             warnings.warn(UserWarning(msg), stacklevel=1)
-        config_dict["write_empty_chunks"] = write_empty_chunks
-    if order is not None and config is not None:
-        msg = (
-            "Both order and config keyword arguments are set. "
-            "This is redundant. When both are set, order will be ignored and "
-            "config will be used."
-        )
-        warnings.warn(UserWarning(msg), stacklevel=1)
-
-    config_parsed = ArrayConfig.from_dict(config_dict)
+        config_parsed = dataclasses.replace(config_parsed, write_empty_chunks=write_empty_chunks)
 
     return await AsyncArray._create(
         store_path,
@@ -1263,8 +1251,6 @@ async def open_array(
 
     zarr_format = _handle_zarr_version_or_format(zarr_version=zarr_version, zarr_format=zarr_format)
 
-    if "order" in kwargs:
-        _warn_order_kwarg()
     if "write_empty_chunks" in kwargs:
         _warn_write_empty_chunks_kwarg()
 

src/zarr/core/array.py

@@ -62,7 +62,6 @@
     _warn_order_kwarg,
     concurrent_map,
     parse_dtype,
-    parse_order,
     parse_shapelike,
     product,
 )
@@ -603,7 +602,6 @@ async def _create(
 
             if order is not None:
                 _warn_order_kwarg()
-                config_parsed = replace(config_parsed, order=order)
 
             result = await cls._create_v3(
                 store_path,
@@ -631,9 +629,10 @@ async def _create(
                 raise ValueError("dimension_names cannot be used for arrays with zarr_format 2.")
 
             if order is None:
-                order_parsed = parse_order(zarr_config.get("array.order"))
+                order_parsed = config_parsed.order
             else:
                 order_parsed = order
+                config_parsed = replace(config_parsed, order=order)
 
             result = await cls._create_v2(
                 store_path,
@@ -4267,10 +4266,8 @@ async def init_array(
             chunks_out = chunk_shape_parsed
             codecs_out = sub_codecs
 
-        if config is None:
-            config = {}
-        if order is not None and isinstance(config, dict):
-            config["order"] = config.get("order", order)
+        if order is not None:
+            _warn_order_kwarg()
 
         meta = AsyncArray._create_metadata_v3(
             shape=shape_parsed,
@@ -4521,8 +4518,18 @@ def _parse_keep_array_attr(
                 serializer = "auto"
         if fill_value is None:
             fill_value = data.fill_value
-        if order is None:
+
+        if data.metadata.zarr_format == 2 and zarr_format == 3 and data.order == "F":
+            # Can't set order="F" for v3 arrays
+            warnings.warn(
+                "Zarr format 3 arrays are always stored with order='C'. "
+                "The existing order='F' of the source Zarr format 2 array will be ignored.",
+                UserWarning,
+                stacklevel=2,
+            )
+        elif order is None and zarr_format == 2:
             order = data.order
+
         if chunk_key_encoding is None and zarr_format == data.metadata.zarr_format:
             if isinstance(data.metadata, ArrayV2Metadata):
                 chunk_key_encoding = {"name": "v2", "separator": data.metadata.dimension_separator}

src/zarr/core/group.py

@@ -2428,7 +2428,7 @@ def create_array(
         compressor: CompressorLike = "auto",
         serializer: SerializerLike = "auto",
         fill_value: Any | None = 0,
-        order: MemoryOrder | None = "C",
+        order: MemoryOrder | None = None,
         attributes: dict[str, JSON] | None = None,
         chunk_key_encoding: ChunkKeyEncodingLike | None = None,
         dimension_names: DimensionNames = None,
@@ -2822,7 +2822,7 @@ def array(
         compressor: CompressorLike = None,
         serializer: SerializerLike = "auto",
         fill_value: Any | None = 0,
-        order: MemoryOrder | None = "C",
+        order: MemoryOrder | None = None,
         attributes: dict[str, JSON] | None = None,
         chunk_key_encoding: ChunkKeyEncodingLike | None = None,
         dimension_names: DimensionNames = None,

test.py

@@ -0,0 +1,11 @@
+import zarr
+
+# Write fortran style array
+group = zarr.group(store={})
+array = group.create_array(
+    name="example",
+    shape=(128, 128),
+    dtype="float32",
+    order="F",
+    dimension_names=("row", "col"),
+)

tests/test_api.py

@@ -340,34 +340,52 @@ def test_open_with_mode_w_minus(tmp_path: pathlib.Path) -> None:
         zarr.open(store=tmp_path, mode="w-")
 
 
-def test_array_order(zarr_format: ZarrFormat) -> None:
-    arr = zarr.ones(shape=(2, 2), order=None, zarr_format=zarr_format)
-    expected = zarr.config.get("array.order")
-    assert arr.order == expected
+@pytest.mark.parametrize("order", ["C", "F", None])
+@pytest.mark.parametrize("config", [{"order": "C"}, {"order": "F"}, {}], ids=["C", "F", "None"])
+def test_array_order(
+    order: MemoryOrder | None, config: dict[str, MemoryOrder | None], zarr_format: ZarrFormat
+) -> None:
+    """
+    Check that:
+    - For v2, memory order is taken from the `order` keyword argument.
+    - For v3, memory order is taken from `config`, and when order is passed a warning is raised
+    - The numpy array returned has the expected order
+    - For v2, the order metadata is set correctly
+    """
+    default_order = zarr.config.get("array.order")
+    ctx: contextlib.AbstractContextManager  # type: ignore[type-arg]
 
-    vals = np.asarray(arr)
-    if expected == "C":
-        assert vals.flags.c_contiguous
-    elif expected == "F":
-        assert vals.flags.f_contiguous
-    else:
-        raise AssertionError
+    if zarr_format == 3:
+        if order is None:
+            ctx = contextlib.nullcontext()
+        else:
+            ctx = pytest.warns(
+                RuntimeWarning,
+                match="The `order` keyword argument has no effect for Zarr format 3 arrays",
+            )
 
+        expected_order = config.get("order", default_order)
 
-@pytest.mark.parametrize("order", ["C", "F"])
-def test_array_order_warns(order: MemoryOrder | None, zarr_format: ZarrFormat) -> None:
-    with pytest.warns(RuntimeWarning, match="The `order` keyword argument .*"):
-        arr = zarr.ones(shape=(2, 2), order=order, zarr_format=zarr_format)
-    assert arr.order == order
+    if zarr_format == 2:
+        ctx = contextlib.nullcontext()
+        expected_order = order or config.get("order", default_order)
 
+    with ctx:
+        arr = zarr.ones(shape=(2, 2), order=order, zarr_format=zarr_format, config=config)
+
+    assert arr.order == expected_order
     vals = np.asarray(arr)
-    if order == "C":
+    if expected_order == "C":
         assert vals.flags.c_contiguous
-    elif order == "F":
+    elif expected_order == "F":
         assert vals.flags.f_contiguous
     else:
         raise AssertionError
 
+    if zarr_format == 2:
+        assert arr.metadata.zarr_format == 2
+        assert arr.metadata.order == expected_order
+
 
 # def test_lazy_loader():
 #     foo = np.arange(100)