Skip to content

Commit

Permalink
Merge branch 'main' into hypothesis-strategies-variable
Browse files Browse the repository at this point in the history
  • Loading branch information
TomNicholas committed Nov 3, 2023
2 parents 2418a61 + 83fbcf0 commit 331f521
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 14 deletions.
5 changes: 5 additions & 0 deletions doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ Bug fixes
(:issue:`8290`, :pull:`8297`).
By `Oliver McCormack <https://github.com/olimcc>`_.

- Fix to_zarr ending in a ReadOnlyError when consolidated metadata was used and the
write_empty_chunks was provided.
(:issue:`8323`, :pull:`8326`)
By `Matthijs Amesz <https://github.com/Metamess>`_.


Documentation
~~~~~~~~~~~~~
Expand Down
17 changes: 16 additions & 1 deletion xarray/backends/zarr.py
Original file line number Diff line number Diff line change
Expand Up @@ -675,8 +675,23 @@ def set_variables(self, variables, check_encoding_set, writer, unlimited_dims=No
# metadata. This would need some case work properly with region
# and append_dim.
if self._write_empty is not None:
# Write to zarr_group.chunk_store instead of zarr_group.store
# See https://github.com/pydata/xarray/pull/8326#discussion_r1365311316 for a longer explanation
# The open_consolidated() enforces a mode of r or r+
# (and to_zarr with region provided enforces a read mode of r+),
# and this function makes sure the resulting Group has a store of type ConsolidatedMetadataStore
# and a 'normal Store subtype for chunk_store.
# The exact type depends on if a local path was used, or a URL of some sort,
# but the point is that it's not a read-only ConsolidatedMetadataStore.
# It is safe to write chunk data to the chunk_store because no metadata would be changed by
# to_zarr with the region parameter:
# - Because the write mode is enforced to be r+, no new variables can be added to the store
# (this is also checked and enforced in xarray.backends.api.py::to_zarr()).
# - Existing variables already have their attrs included in the consolidated metadata file.
# - The size of dimensions can not be expanded, that would require a call using `append_dim`
# which is mutually exclusive with `region`
zarr_array = zarr.open(
store=self.zarr_group.store,
store=self.zarr_group.chunk_store,
path=f"{self.zarr_group.name}/{name}",
write_empty_chunks=self._write_empty,
)
Expand Down
13 changes: 6 additions & 7 deletions xarray/namedarray/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@

from xarray.core.types import Dims
from xarray.namedarray._typing import (
DuckArray,
_AttrsLike,
_Chunks,
_Dim,
Expand Down Expand Up @@ -144,9 +143,9 @@ def _new(
@overload
def from_array(
dims: _DimsLike,
data: DuckArray[_ScalarType],
data: duckarray[_ShapeType, _DType],
attrs: _AttrsLike = ...,
) -> _NamedArray[_ScalarType]:
) -> NamedArray[_ShapeType, _DType]:
...


Expand All @@ -155,15 +154,15 @@ def from_array(
dims: _DimsLike,
data: ArrayLike,
attrs: _AttrsLike = ...,
) -> _NamedArray[Any]:
) -> NamedArray[Any, Any]:
...


def from_array(
dims: _DimsLike,
data: DuckArray[_ScalarType] | ArrayLike,
data: duckarray[_ShapeType, _DType] | ArrayLike,
attrs: _AttrsLike = None,
) -> _NamedArray[_ScalarType] | _NamedArray[Any]:
) -> NamedArray[_ShapeType, _DType] | NamedArray[Any, Any]:
"""
Create a Named array from an array-like object.
Expand All @@ -184,7 +183,7 @@ def from_array(
"Array is already a Named array. Use 'data.data' to retrieve the data array"
)

# TODO: dask.array.ma.masked_array also exists, better way?
# TODO: dask.array.ma.MaskedArray also exists, better way?
if isinstance(data, np.ma.MaskedArray):
mask = np.ma.getmaskarray(data) # type: ignore[no-untyped-call]
if mask.any():
Expand Down
13 changes: 9 additions & 4 deletions xarray/tests/test_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -2459,7 +2459,8 @@ def test_no_warning_from_open_emptydim_with_chunks(self) -> None:
@pytest.mark.parametrize("consolidated", [False, True, None])
@pytest.mark.parametrize("compute", [False, True])
@pytest.mark.parametrize("use_dask", [False, True])
def test_write_region(self, consolidated, compute, use_dask) -> None:
@pytest.mark.parametrize("write_empty", [False, True, None])
def test_write_region(self, consolidated, compute, use_dask, write_empty) -> None:
if (use_dask or not compute) and not has_dask:
pytest.skip("requires dask")
if consolidated and self.zarr_version > 2:
Expand Down Expand Up @@ -2491,6 +2492,7 @@ def test_write_region(self, consolidated, compute, use_dask) -> None:
store,
region=region,
consolidated=consolidated,
write_empty_chunks=write_empty,
**self.version_kwargs,
)
with xr.open_zarr(
Expand Down Expand Up @@ -2772,9 +2774,12 @@ def roundtrip_dir(
) as ds:
yield ds

@pytest.mark.parametrize("write_empty", [True, False])
def test_write_empty(self, write_empty: bool) -> None:
if not write_empty:
@pytest.mark.parametrize("consolidated", [True, False, None])
@pytest.mark.parametrize("write_empty", [True, False, None])
def test_write_empty(
self, consolidated: bool | None, write_empty: bool | None
) -> None:
if write_empty is False:
expected = ["0.1.0", "1.1.0"]
else:
expected = [
Expand Down
8 changes: 6 additions & 2 deletions xarray/tests/test_namedarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,10 +290,14 @@ def test_duck_array_typevar(a: duckarray[Any, _DType]) -> duckarray[Any, _DType]

numpy_a: NDArray[np.int64]
numpy_a = np.array([2.1, 4], dtype=np.dtype(np.int64))
test_duck_array_typevar(numpy_a)

masked_a: np.ma.MaskedArray[Any, np.dtype[np.int64]]
masked_a = np.ma.asarray([2.1, 4], dtype=np.dtype(np.int64)) # type: ignore[no-untyped-call]
test_duck_array_typevar(masked_a)

custom_a: CustomArrayIndexable[Any, np.dtype[np.int64]]
custom_a = CustomArrayIndexable(numpy_a)

test_duck_array_typevar(numpy_a)
test_duck_array_typevar(custom_a)

# Test numpy's array api:
Expand Down

0 comments on commit 331f521

Please sign in to comment.