Skip to content

Don't warn when order passed for v2 arrays #3078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 3 additions & 28 deletions src/zarr/api/asynchronous.py
Original file line number Diff line number Diff line change
@@ -17,7 +17,6 @@
from_array,
get_array_metadata,
)
from zarr.core.array_spec import ArrayConfig, ArrayConfigLike, ArrayConfigParams
from zarr.core.buffer import NDArrayLike
from zarr.core.common import (
JSON,
@@ -27,7 +26,6 @@
MemoryOrder,
ZarrFormat,
_default_zarr_format,
_warn_order_kwarg,
_warn_write_empty_chunks_kwarg,
parse_dtype,
)
@@ -46,6 +44,7 @@
from collections.abc import Iterable

from zarr.abc.codec import Codec
from zarr.core.array_spec import ArrayConfigLike
from zarr.core.buffer import NDArrayLikeOrScalar
from zarr.core.chunk_key_encodings import ChunkKeyEncoding
from zarr.storage import StoreLike
@@ -1010,8 +1009,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()

@@ -1023,27 +1020,6 @@ async def create(
mode = "a"
store_path = await make_store_path(store, path=path, mode=mode, storage_options=storage_options)

config_dict: ArrayConfigParams = {}

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."
)
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)

return await AsyncArray._create(
store_path,
shape=shape,
@@ -1060,8 +1036,9 @@ async def create(
chunk_key_encoding=chunk_key_encoding,
codecs=codecs,
dimension_names=dimension_names,
write_empty_chunks=write_empty_chunks,
attributes=attributes,
config=config_parsed,
config=config,
**kwargs,
)

@@ -1245,8 +1222,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()

61 changes: 43 additions & 18 deletions src/zarr/core/array.py
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@
from zarr.abc.store import Store, set_or_delete
from zarr.codecs._v2 import V2Codec
from zarr.core._info import ArrayInfo
from zarr.core.array_spec import ArrayConfig, ArrayConfigLike, parse_array_config
from zarr.core.array_spec import ArrayConfig, ArrayConfigLike, ArrayConfigParams, parse_array_config
from zarr.core.attributes import Attributes
from zarr.core.buffer import (
BufferPrototype,
@@ -62,7 +62,6 @@
_warn_order_kwarg,
concurrent_map,
parse_dtype,
parse_order,
parse_shapelike,
product,
)
@@ -564,6 +563,7 @@
filters: list[dict[str, JSON]] | None = None,
compressor: CompressorLike = "auto",
# runtime
write_empty_chunks: bool | None = None,
overwrite: bool = False,
data: npt.ArrayLike | None = None,
config: ArrayConfigLike | None = None,
@@ -584,7 +584,34 @@
_chunks = normalize_chunks(chunks, shape, dtype_parsed.itemsize)
else:
_chunks = normalize_chunks(chunk_shape, shape, dtype_parsed.itemsize)
config_parsed = parse_array_config(config)

if config is not None:
config_parsed = parse_array_config(config)
if write_empty_chunks is not None:
msg = (

Check warning on line 591 in src/zarr/core/array.py

Codecov / codecov/patch

src/zarr/core/array.py#L589-L591

Added lines #L589 - L591 were not covered by tests
"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."
)
warnings.warn(UserWarning(msg), stacklevel=1)
if order is not None:
msg = (

Check warning on line 598 in src/zarr/core/array.py

Codecov / codecov/patch

src/zarr/core/array.py#L596-L598

Added lines #L596 - L598 were not covered by tests
"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)

Check warning on line 603 in src/zarr/core/array.py

Codecov / codecov/patch

src/zarr/core/array.py#L603

Added line #L603 was not covered by tests
else:
config_dict: ArrayConfigParams = {}
if write_empty_chunks is not None:
config_dict["write_empty_chunks"] = write_empty_chunks

Check warning on line 607 in src/zarr/core/array.py

Codecov / codecov/patch

src/zarr/core/array.py#L607

Added line #L607 was not covered by tests
if order is not None:
if zarr_format == 3:
_warn_order_kwarg()

Check warning on line 610 in src/zarr/core/array.py

Codecov / codecov/patch

src/zarr/core/array.py#L609-L610

Added lines #L609 - L610 were not covered by tests
else:
config_dict["order"] = order

Check warning on line 612 in src/zarr/core/array.py

Codecov / codecov/patch

src/zarr/core/array.py#L612

Added line #L612 was not covered by tests

config_parsed = ArrayConfig.from_dict(config_dict)

result: AsyncArray[ArrayV3Metadata] | AsyncArray[ArrayV2Metadata]
if zarr_format == 3:
@@ -601,10 +628,6 @@
"compressor cannot be used for arrays with zarr_format 3. Use bytes-to-bytes codecs instead."
)

if order is not None:
_warn_order_kwarg()
config_parsed = replace(config_parsed, order=order)

result = await cls._create_v3(
store_path,
shape=shape,
@@ -630,19 +653,13 @@
if dimension_names is not None:
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"))
else:
order_parsed = order

result = await cls._create_v2(
store_path,
shape=shape,
dtype=dtype_parsed,
chunks=_chunks,
dimension_separator=dimension_separator,
fill_value=fill_value,
order=order_parsed,
config=config_parsed,
filters=filters,
compressor=compressor,
@@ -788,7 +805,6 @@
shape: ChunkCoords,
dtype: np.dtype[Any],
chunks: ChunkCoords,
order: MemoryOrder,
config: ArrayConfig,
dimension_separator: Literal[".", "/"] | None = None,
fill_value: float | None = None,
@@ -820,7 +836,7 @@
shape=shape,
dtype=dtype,
chunks=chunks,
order=order,
order=config.order,
dimension_separator=dimension_separator,
fill_value=fill_value,
filters=filters,
@@ -1041,6 +1057,14 @@
-------
bool
Memory order of the array

Notes
-----
For Zarr format 2 arrays this is the memory order in which
data is stored and returned when read in to a NumPy array.

For Zarr format 3 arrays this is just the memory order
in which data is returned when read into a NumPy array.
"""
if self.metadata.zarr_format == 2:
return self.metadata.order
@@ -4239,6 +4263,7 @@
attributes=attributes,
)
else:
# zarr_format == 3
array_array, array_bytes, bytes_bytes = _parse_chunk_encoding_v3(
compressors=compressors,
filters=filters,
@@ -4269,8 +4294,8 @@

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()

Check warning on line 4298 in src/zarr/core/array.py

Codecov / codecov/patch

src/zarr/core/array.py#L4298

Added line #L4298 was not covered by tests

meta = AsyncArray._create_metadata_v3(
shape=shape_parsed,
@@ -4521,7 +4546,7 @@
serializer = "auto"
if fill_value is None:
fill_value = data.fill_value
if order is None:
if order is None and zarr_format == 2:

Check warning on line 4549 in src/zarr/core/array.py

Codecov / codecov/patch

src/zarr/core/array.py#L4549

Added line #L4549 was not covered by tests
order = data.order
if chunk_key_encoding is None and zarr_format == data.metadata.zarr_format:
if isinstance(data.metadata, ArrayV2Metadata):
4 changes: 2 additions & 2 deletions src/zarr/core/group.py
Original file line number Diff line number Diff line change
@@ -2380,7 +2380,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,
@@ -2774,7 +2774,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,
53 changes: 41 additions & 12 deletions tests/test_api.py
Original file line number Diff line number Diff line change
@@ -353,19 +353,28 @@ def test_array_order(zarr_format: ZarrFormat) -> None:
raise AssertionError


@pytest.mark.parametrize("config_order", ["C", "F"])
@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

vals = np.asarray(arr)
if order == "C":
assert vals.flags.c_contiguous
elif order == "F":
assert vals.flags.f_contiguous
else:
raise AssertionError
def test_array_order_warns(
order: MemoryOrder, config_order: MemoryOrder, zarr_format: ZarrFormat
) -> None:
with zarr.config.set({"array.order": config_order}):
if zarr_format == 3:
with pytest.warns(RuntimeWarning, match="The `order` keyword argument .*"):
arr = zarr.ones(shape=(2, 2), order=order, zarr_format=zarr_format)
# For zarr v3, order should be ignored and taken from config
assert arr.order == config_order
else:
arr = zarr.ones(shape=(2, 2), order=order, zarr_format=zarr_format)
assert arr.order == order

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


# def test_lazy_loader():
@@ -1339,3 +1348,23 @@ def test_auto_chunks(f: Callable[..., Array]) -> None:

a = f(**kwargs)
assert a.chunks == (500, 500)


def test_order_warning() -> None:
# Passing order shouldn't warn for v2
zarr.create(
(1,),
store={},
order="F",
zarr_format=2,
)
# Passing order should warn for v3
with pytest.warns(
RuntimeWarning, match="The `order` keyword argument has no effect for Zarr format 3 arrays"
):
zarr.create(
(1,),
store={},
order="F",
zarr_format=3,
)
26 changes: 18 additions & 8 deletions tests/test_array.py
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
import pickle
import re
import sys
from contextlib import nullcontext
from itertools import accumulate
from typing import TYPE_CHECKING, Any, Literal
from unittest import mock
@@ -1425,14 +1426,23 @@ def test_order(
# 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 == 3 and order is not None:
ctx = pytest.warns(
RuntimeWarning,
match="The `order` keyword argument has no effect for Zarr format 3 arrays",
)
else:
ctx = nullcontext()

with ctx:
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:
assert arr.metadata.zarr_format == 2