Skip to content
Merged
10 changes: 3 additions & 7 deletions src/zarr/api/asynchronous.py
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ async def create(
dtype: npt.DTypeLike | None = None,
compressor: dict[str, JSON] | None = None, # TODO: default and type change
fill_value: Any | None = 0, # TODO: need type
order: MemoryOrder | None = None, # TODO: default change
order: MemoryOrder | None = None,
store: str | StoreLike | None = None,
synchronizer: Any | None = None,
overwrite: bool = False,
Expand Down Expand Up @@ -761,6 +761,7 @@ async def create(
Default value to use for uninitialized portions of the array.
order : {'C', 'F'}, optional
Memory layout to be used within each chunk.
Default is set in Zarr's config (`array.order`).
store : Store or str
Store or path to directory in file system or name of zip file.
synchronizer : object, optional
Expand Down Expand Up @@ -834,12 +835,6 @@ async def create(
else:
chunk_shape = shape

if order is not None:
warnings.warn(
"order is deprecated, use config `array.order` instead",
DeprecationWarning,
stacklevel=2,
)
if synchronizer is not None:
warnings.warn("synchronizer is not yet implemented", RuntimeWarning, stacklevel=2)
if chunk_store is not None:
Expand Down Expand Up @@ -889,6 +884,7 @@ async def create(
codecs=codecs,
dimension_names=dimension_names,
attributes=attributes,
order=order,
**kwargs,
)

Expand Down
12 changes: 5 additions & 7 deletions src/zarr/core/array.py
Original file line number Diff line number Diff line change
Expand Up @@ -471,10 +471,6 @@ async def create(
raise ValueError(
"dimension_separator cannot be used for arrays with version 3. Use chunk_key_encoding instead."
)
if order is not None:
raise ValueError(
"order cannot be used for arrays with version 3. Use a transpose codec instead."
)
if filters is not None:
raise ValueError(
"filters cannot be used for arrays with version 3. Use array-to-array codecs instead."
Expand All @@ -494,6 +490,7 @@ async def create(
dimension_names=dimension_names,
attributes=attributes,
exists_ok=exists_ok,
order=order,
)
elif zarr_format == 2:
if dtype is str or dtype == "str":
Expand Down Expand Up @@ -545,6 +542,7 @@ async def _create_v3(
dtype: npt.DTypeLike,
chunk_shape: ChunkCoords,
fill_value: Any | None = None,
order: Literal["C", "F"] | None = None,
chunk_key_encoding: (
ChunkKeyEncoding
| tuple[Literal["default"], Literal[".", "/"]]
Expand Down Expand Up @@ -588,7 +586,7 @@ async def _create_v3(
attributes=attributes or {},
)

array = cls(metadata=metadata, store_path=store_path)
array = cls(metadata=metadata, store_path=store_path, order=order)
await array._save_metadata(metadata, ensure_parents=True)
return array

Expand All @@ -611,7 +609,7 @@ async def _create_v2(
if not exists_ok:
await ensure_no_existing_node(store_path, zarr_format=2)
if order is None:
order = "C"
order = config.get("array.order", "C")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suppose we change the key array.order to array_indexing.order in the config, this will happily return "C", when the correct behavior should be an error. Therefore I think we should define the default value higher up than this function, and this function should have a hard dependency on the schema of the config.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow. What's the ask here?

Copy link
Contributor

@d-v-b d-v-b Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the "default array order is C" logic should be defined higher up in the stack, instead of in this function. so the change would be to a) do that, and b) make this line order = config.get("array.order"), i.e. something that will error if we change the schema of the config.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much higher? We could require passing order into Array.create rather than allowing this to be a nullable field. Or we could do the config check there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be possible to put the default in the config? I'm guessing we will have a lot of defaults (e.g., codecs), so would probably make sense to put them all in the same place

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like the defaults are already in the config, in which case I think we can just remove any "default to C" behavior in _create_v2

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay. I think the final state should be what you are after. Because the v2 array metadata requires order, we do have to check the config setting if no order was passed, but we no longer apply a default on top of that.


if dimension_separator is None:
dimension_separator = "."
Expand All @@ -627,7 +625,7 @@ async def _create_v2(
filters=filters,
attributes=attributes,
)
array = cls(metadata=metadata, store_path=store_path)
array = cls(metadata=metadata, store_path=store_path, order=order)
await array._save_metadata(metadata, ensure_parents=True)
return array

Expand Down
16 changes: 16 additions & 0 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,22 @@ def test_open_with_mode_w_minus(tmp_path: pathlib.Path) -> None:
zarr.open(store=tmp_path, mode="w-")


@pytest.mark.parametrize("order", ["C", "F", None])
@pytest.mark.parametrize("zarr_format", [2, 3])
def test_array_order(order: str | None, zarr_format: int) -> None:
arr = zarr.ones(shape=(2, 2), order=order, zarr_format=zarr_format)
expected = order or zarr.config.get("array.order")
assert arr.order == expected

vals = np.asarray(arr)
if expected == "C":
assert vals.flags.c_contiguous
elif expected == "F":
assert vals.flags.f_contiguous
else:
raise AssertionError


# def test_lazy_loader():
# foo = np.arange(100)
# bar = np.arange(100, 0, -1)
Expand Down
17 changes: 17 additions & 0 deletions tests/test_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,20 @@ def test_update_attrs(zarr_format: int) -> None:

arr2 = zarr.open_array(store=store, zarr_format=zarr_format)
assert arr2.attrs["foo"] == "bar"


@pytest.mark.parametrize("order", ["C", "F", None])
@pytest.mark.parametrize("zarr_format", [2, 3])
@pytest.mark.parametrize("store", ["memory"], indirect=True)
def test_array_create_order(order: str | None, zarr_format: int, store: MemoryStore) -> None:
arr = Array.create(store=store, shape=(2, 2), order=order, zarr_format=zarr_format, dtype="i4")
expected = order or zarr.config.get("array.order")
assert arr.order == expected

vals = np.asarray(arr)
if expected == "C":
assert vals.flags.c_contiguous
elif expected == "F":
assert vals.flags.f_contiguous
else:
raise AssertionError