From a31b0d244763887684ce8fb80489232e2f0a1ee4 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Thu, 3 Apr 2025 19:31:34 +0100 Subject: [PATCH 01/11] Fix specifying memory order in v2 arrays --- src/zarr/api/asynchronous.py | 17 ++++++++--------- src/zarr/core/array.py | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) 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..31e9336e4d 100644 --- a/src/zarr/core/array.py +++ b/src/zarr/core/array.py @@ -1044,7 +1044,7 @@ def order(self) -> MemoryOrder: bool Memory order of the array """ - return self._config.order + return self.metadata.order @property def attrs(self) -> dict[str, JSON]: From 06f14e8fc2a6facd3a1a09884ff5679b48a9ec02 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Thu, 3 Apr 2025 19:42:08 +0100 Subject: [PATCH 02/11] Re-work test_order --- tests/test_array.py | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/tests/test_array.py b/tests/test_array.py index 5c3c556dfb..2680cdf5b6 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: @@ -1273,28 +1275,29 @@ def test_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: + 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, + ) 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 - ) + assert arr.order == expected + vals = np.asarray(arr) if expected == "C": assert vals.flags.c_contiguous From bf67584210a53602b4905189f5a742acbd55d11e Mon Sep 17 00:00:00 2001 From: David Stansby Date: Thu, 3 Apr 2025 20:57:27 +0100 Subject: [PATCH 03/11] Fix getting array order in v3 --- src/zarr/core/array.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/zarr/core/array.py b/src/zarr/core/array.py index 31e9336e4d..d3ee994bcc 100644 --- a/src/zarr/core/array.py +++ b/src/zarr/core/array.py @@ -1044,7 +1044,10 @@ def order(self) -> MemoryOrder: bool Memory order of the array """ - return self.metadata.order + if self.metadata.zarr_format == 2: + return self.metadata.order + else: + return self._config.order @property def attrs(self) -> dict[str, JSON]: From 56cc68171e7db4958702568f95b2deede3b38970 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Thu, 3 Apr 2025 20:59:38 +0100 Subject: [PATCH 04/11] Fix order of arrays for v2 --- src/zarr/core/array.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/zarr/core/array.py b/src/zarr/core/array.py index d3ee994bcc..d57dd8bb34 100644 --- a/src/zarr/core/array.py +++ b/src/zarr/core/array.py @@ -1279,14 +1279,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( From 3794e36d6df757b6083471c36adeacbdca590ef1 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Thu, 3 Apr 2025 21:06:47 +0100 Subject: [PATCH 05/11] Fix order with V3 arrays --- src/zarr/core/array.py | 6 ++++++ tests/test_array.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/zarr/core/array.py b/src/zarr/core/array.py index d57dd8bb34..27bbc744b4 100644 --- a/src/zarr/core/array.py +++ b/src/zarr/core/array.py @@ -4259,6 +4259,12 @@ async def init_array( chunks_out = chunk_shape_parsed codecs_out = sub_codecs + if order is not None: + if config is None: + config = {} + if "order" not in config: + config["order"] = order + meta = AsyncArray._create_metadata_v3( shape=shape_parsed, dtype=dtype_parsed, diff --git a/tests/test_array.py b/tests/test_array.py index 2680cdf5b6..badf1f7ad2 100644 --- a/tests/test_array.py +++ b/tests/test_array.py @@ -1294,9 +1294,9 @@ def test_order( order=order, config=config, ) + assert arr.order == expected if zarr_format == 2: assert arr.metadata.order == expected - assert arr.order == expected vals = np.asarray(arr) if expected == "C": From 6e2688b24d3643dbad7c0cf4139b537723f211f9 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Thu, 3 Apr 2025 21:21:04 +0100 Subject: [PATCH 06/11] Fix mypy --- src/zarr/core/array.py | 10 +++++----- tests/test_array.py | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/zarr/core/array.py b/src/zarr/core/array.py index 27bbc744b4..b24ee0f523 100644 --- a/src/zarr/core/array.py +++ b/src/zarr/core/array.py @@ -4259,12 +4259,12 @@ async def init_array( chunks_out = chunk_shape_parsed codecs_out = sub_codecs - if order is not None: - if config is None: - config = {} - if "order" not in config: - config["order"] = order + if config is None: + config = {} + if order is not None and isinstance(config, dict): + config["order"] = config.get("order", order) + print(config) meta = AsyncArray._create_metadata_v3( shape=shape_parsed, dtype=dtype_parsed, diff --git a/tests/test_array.py b/tests/test_array.py index badf1f7ad2..4be9bbde43 100644 --- a/tests/test_array.py +++ b/tests/test_array.py @@ -1274,7 +1274,7 @@ 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 = {} + config: ArrayConfigLike | None = {} if order is None: config = {} expected = zarr.config.get("array.order") @@ -1296,6 +1296,7 @@ def test_order( ) assert arr.order == expected if zarr_format == 2: + assert arr.metadata.zarr_format == 2 assert arr.metadata.order == expected vals = np.asarray(arr) From 64eb3148c0ad42aaeacfae7ec62e4704653f998a Mon Sep 17 00:00:00 2001 From: David Stansby Date: Thu, 3 Apr 2025 21:24:05 +0100 Subject: [PATCH 07/11] Remove errant print() --- src/zarr/core/array.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/zarr/core/array.py b/src/zarr/core/array.py index b24ee0f523..96c48cdedb 100644 --- a/src/zarr/core/array.py +++ b/src/zarr/core/array.py @@ -4264,7 +4264,6 @@ async def init_array( if order is not None and isinstance(config, dict): config["order"] = config.get("order", order) - print(config) meta = AsyncArray._create_metadata_v3( shape=shape_parsed, dtype=dtype_parsed, From 01a55549de86fa7163e570a8fe5e052e3be55a0f Mon Sep 17 00:00:00 2001 From: David Stansby Date: Thu, 3 Apr 2025 22:32:07 +0100 Subject: [PATCH 08/11] Fix order with v3 arrays --- src/zarr/core/array.py | 1 + tests/test_api.py | 7 +++---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/zarr/core/array.py b/src/zarr/core/array.py index 96c48cdedb..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, 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 From 3371dc4feffcf0eb8304ff5e61c767404465d14c Mon Sep 17 00:00:00 2001 From: David Stansby Date: Thu, 3 Apr 2025 22:44:33 +0100 Subject: [PATCH 09/11] Fix v2 test --- tests/test_v2.py | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/tests/test_v2.py b/tests/test_v2.py index 3a36bc01fd..9b72f94a28 100644 --- a/tests/test_v2.py +++ b/tests/test_v2.py @@ -195,12 +195,8 @@ 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("order", ["C", "F"]) +def test_v2_non_contiguous(order: Literal["C", "F"]) -> None: store = MemoryStore() arr = zarr.create_array( store, @@ -212,12 +208,11 @@ def test_v2_non_contiguous( filters=None, compressors=None, overwrite=True, - order=array_order, - config={"order": memory_order}, + order=order, ) # Non-contiguous write - a = np.arange(arr.shape[0] * arr.shape[1]).reshape(arr.shape, order=data_order) + a = np.arange(arr.shape[0] * arr.shape[1]).reshape(arr.shape, order=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,9 +220,9 @@ 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=order), ) - if memory_order == "F": + if order == "F": assert (arr[6:9, 3:6]).flags.f_contiguous else: assert (arr[6:9, 3:6]).flags.c_contiguous @@ -243,13 +238,12 @@ def test_v2_non_contiguous( compressors=None, filters=None, overwrite=True, - order=array_order, - config={"order": memory_order}, + order=order, ) # Contiguous write - a = np.arange(9).reshape((3, 3), order=data_order) - if data_order == "F": + a = np.arange(9).reshape((3, 3), order=order) + if order == "F": assert a.flags.f_contiguous else: assert a.flags.c_contiguous From 9f0a2b265634f23c76fbdc1abbd0cb6f8f95eacf Mon Sep 17 00:00:00 2001 From: David Stansby Date: Tue, 29 Apr 2025 10:23:02 +0100 Subject: [PATCH 10/11] Add numpy order parametrization --- tests/test_v2.py | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/tests/test_v2.py b/tests/test_v2.py index 9b72f94a28..8f0e1b2d29 100644 --- a/tests/test_v2.py +++ b/tests/test_v2.py @@ -195,8 +195,13 @@ def test_create_array_defaults(store: Store): ) -@pytest.mark.parametrize("order", ["C", "F"]) -def test_v2_non_contiguous(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, @@ -208,11 +213,11 @@ def test_v2_non_contiguous(order: Literal["C", "F"]) -> None: filters=None, compressors=None, overwrite=True, - order=order, + order=zarr_order, ) - # Non-contiguous write - a = np.arange(arr.shape[0] * arr.shape[1]).reshape(arr.shape, order=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]) @@ -220,13 +225,15 @@ def test_v2_non_contiguous(order: Literal["C", "F"]) -> None: a[6:9, 3:6], np.frombuffer( sync(store.get("2.1", default_buffer_prototype())).to_bytes(), dtype="float64" - ).reshape((3, 3), order=order), + ).reshape((3, 3), order=zarr_order), ) - if 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, @@ -238,17 +245,17 @@ def test_v2_non_contiguous(order: Literal["C", "F"]) -> None: compressors=None, filters=None, overwrite=True, - order=order, + order=zarr_order, ) - # Contiguous write - a = np.arange(9).reshape((3, 3), order=order) - if 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(): From cdba47dd19b051ffd74dce2785362eeef628e329 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Tue, 29 Apr 2025 10:25:56 +0100 Subject: [PATCH 11/11] Add changelog entry --- changes/2950.bufgix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/2950.bufgix.rst 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.