Fix specifying memory order in v2 arrays (#2951)

* Fix specifying memory order in v2 arrays

* Re-work test_order

* Fix getting array order in v3

* Fix order of arrays for v2

* Fix order with V3 arrays

* Fix mypy

* Remove errant print()

* Fix order with v3 arrays

* Fix v2 test

* Add numpy order parametrization

* Add changelog entry

Author: dstansby

changes/2950.bufgix.rst

@@ -0,0 +1 @@
+Specifying the memory order of Zarr format 2 arrays using the ``order`` keyword argument has been fixed.

src/zarr/api/asynchronous.py

@@ -1040,15 +1040,13 @@ async def create(
             )
             warnings.warn(UserWarning(msg), stacklevel=1)
         config_dict["write_empty_chunks"] = write_empty_chunks
-    if order is not None:
-        if 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_dict["order"] = order
+    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)
 
@@ -1062,6 +1060,7 @@ async def create(
         overwrite=overwrite,
         filters=filters,
         dimension_separator=dimension_separator,
+        order=order,
         zarr_format=zarr_format,
         chunk_shape=chunk_shape,
         chunk_key_encoding=chunk_key_encoding,

src/zarr/core/array.py

@@ -609,6 +609,7 @@ 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,
@@ -1044,7 +1045,10 @@ def order(self) -> MemoryOrder:
         bool
             Memory order of the array
         """
-        return self._config.order
+        if self.metadata.zarr_format == 2:
+            return self.metadata.order
+        else:
+            return self._config.order
 
     @property
     def attrs(self) -> dict[str, JSON]:
@@ -1276,14 +1280,14 @@ async def _get_selection(
             out_buffer = prototype.nd_buffer.create(
                 shape=indexer.shape,
                 dtype=out_dtype,
-                order=self._config.order,
+                order=self.order,
                 fill_value=self.metadata.fill_value,
             )
         if product(indexer.shape) > 0:
             # need to use the order from the metadata for v2
             _config = self._config
             if self.metadata.zarr_format == 2:
-                _config = replace(_config, order=self.metadata.order)
+                _config = replace(_config, order=self.order)
 
             # reading chunks and decoding them
             await self.codec_pipeline.read(
@@ -4256,6 +4260,11 @@ 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)
+
         meta = AsyncArray._create_metadata_v3(
             shape=shape_parsed,
             dtype=dtype_parsed,

tests/test_api.py

@@ -326,13 +326,12 @@ def test_array_order(zarr_format: ZarrFormat) -> None:
 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)
-    expected = order or zarr.config.get("array.order")
-    assert arr.order == expected
+    assert arr.order == order
 
     vals = np.asarray(arr)
-    if expected == "C":
+    if order == "C":
         assert vals.flags.c_contiguous
-    elif expected == "F":
+    elif order == "F":
         assert vals.flags.f_contiguous
     else:
         raise AssertionError

tests/test_array.py

@@ -1262,39 +1262,43 @@ async def test_data_ignored_params(store: Store) -> None:
             await create_array(store, data=data, shape=None, dtype=data.dtype, overwrite=True)
 
     @staticmethod
-    @pytest.mark.parametrize("order_config", ["C", "F", None])
+    @pytest.mark.parametrize("order", ["C", "F", None])
+    @pytest.mark.parametrize("with_config", [True, False])
     def test_order(
-        order_config: MemoryOrder | None,
+        order: MemoryOrder | None,
+        with_config: bool,
         zarr_format: ZarrFormat,
         store: MemoryStore,
     ) -> None:
         """
         Test that the arrays generated by array indexing have a memory order defined by the config order
         value, and that for zarr v2 arrays, the ``order`` field in the array metadata is set correctly.
         """
-        config: ArrayConfigLike = {}
-        if order_config is None:
+        config: ArrayConfigLike | None = {}
+        if order is None:
             config = {}
             expected = zarr.config.get("array.order")
         else:
-            config = {"order": order_config}
-            expected = order_config
+            config = {"order": order}
+            expected = order
+
+        if not with_config:
+            # Test without passing config parameter
+            config = None
+
+        arr = zarr.create_array(
+            store=store,
+            shape=(2, 2),
+            zarr_format=zarr_format,
+            dtype="i4",
+            order=order,
+            config=config,
+        )
+        assert arr.order == expected
         if zarr_format == 2:
-            arr = zarr.create_array(
-                store=store,
-                shape=(2, 2),
-                zarr_format=zarr_format,
-                dtype="i4",
-                order=expected,
-                config=config,
-            )
-            # guard for type checking
             assert arr.metadata.zarr_format == 2
             assert arr.metadata.order == expected
-        else:
-            arr = zarr.create_array(
-                store=store, shape=(2, 2), zarr_format=zarr_format, dtype="i4", config=config
-            )
+
         vals = np.asarray(arr)
         if expected == "C":
             assert vals.flags.c_contiguous

tests/test_v2.py

@@ -195,12 +195,13 @@ def test_create_array_defaults(store: Store):
         )
 
 
-@pytest.mark.parametrize("array_order", ["C", "F"])
-@pytest.mark.parametrize("data_order", ["C", "F"])
-@pytest.mark.parametrize("memory_order", ["C", "F"])
-def test_v2_non_contiguous(
-    array_order: Literal["C", "F"], data_order: Literal["C", "F"], memory_order: Literal["C", "F"]
-) -> None:
+@pytest.mark.parametrize("numpy_order", ["C", "F"])
+@pytest.mark.parametrize("zarr_order", ["C", "F"])
+def test_v2_non_contiguous(numpy_order: Literal["C", "F"], zarr_order: Literal["C", "F"]) -> None:
+    """
+    Make sure zarr v2 arrays save data using the memory order given to the zarr array,
+    not the memory order of the original numpy array.
+    """
     store = MemoryStore()
     arr = zarr.create_array(
         store,
@@ -212,26 +213,27 @@ def test_v2_non_contiguous(
         filters=None,
         compressors=None,
         overwrite=True,
-        order=array_order,
-        config={"order": memory_order},
+        order=zarr_order,
     )
 
-    # Non-contiguous write
-    a = np.arange(arr.shape[0] * arr.shape[1]).reshape(arr.shape, order=data_order)
+    # Non-contiguous write, using numpy memory order
+    a = np.arange(arr.shape[0] * arr.shape[1]).reshape(arr.shape, order=numpy_order)
     arr[6:9, 3:6] = a[6:9, 3:6]  # The slice on the RHS is important
     np.testing.assert_array_equal(arr[6:9, 3:6], a[6:9, 3:6])
 
     np.testing.assert_array_equal(
         a[6:9, 3:6],
         np.frombuffer(
             sync(store.get("2.1", default_buffer_prototype())).to_bytes(), dtype="float64"
-        ).reshape((3, 3), order=array_order),
+        ).reshape((3, 3), order=zarr_order),
     )
-    if memory_order == "F":
+    # After writing and reading from zarr array, order should be same as zarr order
+    if zarr_order == "F":
         assert (arr[6:9, 3:6]).flags.f_contiguous
     else:
         assert (arr[6:9, 3:6]).flags.c_contiguous
 
+    # Contiguous write
     store = MemoryStore()
     arr = zarr.create_array(
         store,
@@ -243,18 +245,17 @@ def test_v2_non_contiguous(
         compressors=None,
         filters=None,
         overwrite=True,
-        order=array_order,
-        config={"order": memory_order},
+        order=zarr_order,
     )
 
-    # Contiguous write
-    a = np.arange(9).reshape((3, 3), order=data_order)
-    if data_order == "F":
-        assert a.flags.f_contiguous
-    else:
-        assert a.flags.c_contiguous
+    a = np.arange(9).reshape((3, 3), order=numpy_order)
     arr[6:9, 3:6] = a
     np.testing.assert_array_equal(arr[6:9, 3:6], a)
+    # After writing and reading from zarr array, order should be same as zarr order
+    if zarr_order == "F":
+        assert (arr[6:9, 3:6]).flags.f_contiguous
+    else:
+        assert (arr[6:9, 3:6]).flags.c_contiguous
 
 
 def test_default_compressor_deprecation_warning():