diff --git a/changes/2950.bufgix.rst b/changes/2950.bufgix.rst new file mode 100644 index 0000000000..67cd61f377 --- /dev/null +++ b/changes/2950.bufgix.rst @@ -0,0 +1 @@ +Specifying the memory order of Zarr format 2 arrays using the ``order`` keyword argument has been fixed. diff --git a/src/zarr/api/asynchronous.py b/src/zarr/api/asynchronous.py index 285d777258..9b8b43a517 100644 --- a/src/zarr/api/asynchronous.py +++ b/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, diff --git a/src/zarr/core/array.py b/src/zarr/core/array.py index 62efe44e4c..b0e8b03cd7 100644 --- a/src/zarr/core/array.py +++ b/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, diff --git a/tests/test_api.py b/tests/test_api.py index f03fd53f7a..9f03a1067a 100644 --- a/tests/test_api.py +++ b/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 diff --git a/tests/test_array.py b/tests/test_array.py index 5c3c556dfb..4be9bbde43 100644 --- a/tests/test_array.py +++ b/tests/test_array.py @@ -1262,9 +1262,11 @@ 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: @@ -1272,29 +1274,31 @@ def test_order( 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 diff --git a/tests/test_v2.py b/tests/test_v2.py index 3a36bc01fd..8f0e1b2d29 100644 --- a/tests/test_v2.py +++ b/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,12 +213,11 @@ 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]) @@ -225,13 +225,15 @@ def test_v2_non_contiguous( 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():