Start consolidated metadata deprecation cycle

xarray/backends/api.py

@@ -2187,7 +2187,15 @@ def to_zarr(
 
     See `Dataset.to_zarr` for full API docs.
     """
-    from xarray.backends.zarr import _choose_default_mode, _get_mappers
+    from xarray.backends.zarr import (
+        _choose_default_mode,
+        _get_mappers,
+        _warn_of_consolidated_metadata_deprecation,
+    )
+
+    _warn_of_consolidated_metadata_deprecation(
+        value=consolidated,
+    )
 
     # validate Dataset keys, DataArray names
     _validate_dataset_names(dataset)

xarray/backends/zarr.py

@@ -44,6 +44,19 @@
     from xarray.core.types import ReadBuffer, ZarrArray, ZarrGroup
 
 
+def _warn_of_consolidated_metadata_deprecation(value: True | False | None) -> None:
+    # see issue https://github.com/pydata/xarray/issues/10122
+    if value is None:
+        emit_user_level_warning(
+            "The default value of the ``consolidated`` argument to zarr IO functions will soon change."
+            "The default value of ``None`` used to mean ``True``, but it will be changed to mean ``False``."
+            "To preserve the same behaviour in future please pass ``consolidated=True`` explicitly."
+            "If you are not reading and writing from high-latency stores (e.g. Zarr v2/v3 format cloud object stores) you can safely ignore this warning."
+            "See https://github.com/pydata/xarray/issues/10122 for more information.",
+            PendingDeprecationWarning,
+        )
+
+
 def _get_mappers(*, storage_options, store, chunk_store):
     # expand str and path-like arguments
     store = _normalize_path(store)
@@ -1498,6 +1511,10 @@ def open_zarr(
             "open_zarr() got unexpected keyword arguments " + ",".join(kwargs.keys())
         )
 
+    _warn_of_consolidated_metadata_deprecation(
+        value=consolidated,
+    )
+
     backend_kwargs = {
         "synchronizer": synchronizer,
         "consolidated": consolidated,
@@ -1774,6 +1791,8 @@ def _get_open_params(
     else:
         missing_exc = zarr.errors.GroupNotFoundError
 
+    _warn_of_consolidated_metadata_deprecation(value=consolidated)
+
     if consolidated in [None, True]:
         # open the root of the store, in case there is metadata consolidated there
         group = open_kwargs.pop("path")

xarray/core/datatree.py

@@ -1737,7 +1737,7 @@ def to_zarr(
         store,
         mode: ZarrWriteModes = "w-",
         encoding=None,
-        consolidated: bool = True,
+        consolidated: bool | None = None,
         group: str | None = None,
         write_inherited_coords: bool = False,
         compute: bool = True,

xarray/core/datatree_io.py

@@ -84,7 +84,7 @@ def _datatree_to_zarr(
     store: ZarrStoreLike,
     mode: ZarrWriteModes = "w-",
     encoding: Mapping[str, Any] | None = None,
-    consolidated: bool = True,
+    consolidated: bool | None = None,
     group: str | None = None,
     write_inherited_coords: bool = False,
     compute: bool = True,
@@ -98,6 +98,8 @@ def _datatree_to_zarr(
 
     from zarr import consolidate_metadata
 
+    from xarray.backends.zarr import _warn_of_consolidated_metadata_deprecation
+
     if group is not None:
         raise NotImplementedError(
             "specifying a root group for the tree has not been implemented"
@@ -130,5 +132,9 @@ def _datatree_to_zarr(
         if "w" in mode:
             mode = "a"
 
-    if consolidated:
+    _warn_of_consolidated_metadata_deprecation(
+        value=consolidated,
+    )
+
+    if consolidated in [True, None]:
         consolidate_metadata(store)

xarray/tests/test_backends.py

@@ -36,7 +36,10 @@
     load_dataset,
     open_dataarray,
     open_dataset,
+    open_datatree,
+    open_groups,
     open_mfdataset,
+    open_zarr,
     save_mfdataset,
 )
 from xarray.backends.common import robust_getitem
@@ -506,7 +509,7 @@
     @pytest.mark.filterwarnings("ignore:deallocating CachingFileManager")
     def test_pickle_dataarray(self) -> None:
         expected = Dataset({"foo": ("x", [42])})
         with self.roundtrip(expected, allow_cleanup_failure=ON_WINDOWS) as roundtripped:
             with roundtripped:
                 raw_pickle = pickle.dumps(roundtripped["foo"])
             # TODO: figure out how to explicitly close the file for the
@@ -531,7 +534,7 @@
     @pytest.mark.filterwarnings("ignore:deallocating CachingFileManager")
     def test_roundtrip_None_variable(self) -> None:
         expected = Dataset({None: (("x", "y"), [[0, 1], [2, 3]])})
         with self.roundtrip(expected) as actual:
             assert_identical(expected, actual)
 
     def test_roundtrip_object_dtype(self) -> None:
@@ -555,7 +558,7 @@
             }
         )
         expected = original.copy(deep=True)
         with self.roundtrip(original) as actual:
             try:
                 assert_identical(expected, actual)
             except AssertionError:
@@ -572,7 +575,7 @@
 
     def test_roundtrip_string_data(self) -> None:
         expected = Dataset({"x": ("t", ["ab", "cdef"])})
         with self.roundtrip(expected) as actual:
             assert_identical(expected, actual)
 
     def test_roundtrip_string_encoded_characters(self) -> None:
@@ -710,7 +713,7 @@
 
     def test_orthogonal_indexing(self) -> None:
         in_memory = create_test_data()
         with self.roundtrip(in_memory) as on_disk:
             indexers = {"dim1": [1, 2, 0], "dim2": [3, 2, 0, 3], "dim3": np.arange(5)}
             expected = in_memory.isel(indexers)
             actual = on_disk.isel(**indexers)
@@ -724,7 +727,7 @@
 
     def test_vectorized_indexing(self) -> None:
         in_memory = create_test_data()
         with self.roundtrip(in_memory) as on_disk:
             indexers = {
                 "dim1": DataArray([0, 2, 0], dims="a"),
                 "dim2": DataArray([0, 2, 3], dims="a"),
@@ -814,7 +817,7 @@
                 "dim3": slice(-1, 1, -1),
             }
         ]
         multiple_indexing(indexers)
 
         # with negative step slice.
         indexers = [
@@ -831,7 +834,7 @@
             {"z": (("t", "p", "y", "x"), np.ones((1, 1, 31, 40)))},
         )
 
         with self.roundtrip(ds) as on_disk:
             subset = on_disk.isel(t=[0], p=0).z[:, ::10, ::10][:, ::-1, :]
             assert subset.sizes == subset.load().sizes
 
@@ -1062,7 +1065,7 @@
         encoded = Dataset({"x": ("t", sb, attributes)})
         unsigned_dtype = np.dtype(f"u{sb.dtype.itemsize}")
 
         with _roundtrip_with_warnings(decoded) as actual:
             for k in decoded.variables:
                 assert decoded.variables[k].dtype == actual.variables[k].dtype
                 exp_fv = decoded.variables[k].encoding["_FillValue"]
@@ -1138,7 +1141,7 @@
 
     def test_grid_mapping_and_bounds_are_not_coordinates_in_file(self) -> None:
         original = self._create_cf_dataset()
         with self.roundtrip(original, open_kwargs={"decode_coords": False}) as ds:
             assert ds.coords["latitude"].attrs["bounds"] == "latitude_bnds"
             assert ds.coords["longitude"].attrs["bounds"] == "longitude_bnds"
             assert "coordinates" not in ds["variable"].attrs
@@ -1146,7 +1149,7 @@
 
     def test_coordinate_variables_after_dataset_roundtrip(self) -> None:
         original = self._create_cf_dataset()
         with self.roundtrip(original, open_kwargs={"decode_coords": "all"}) as actual:
             assert_identical(actual, original)
 
         with self.roundtrip(original) as actual:
@@ -1203,7 +1206,7 @@
             {"temp": ("x", [0, 1]), "precip": ("x", [0, -1])},
             {"lat": ("x", [2, 3]), "lon": ("x", [4, 5])},
         )
         with self.roundtrip(original) as actual:
             assert_identical(actual, original)
         with self.roundtrip(original, open_kwargs=dict(decode_coords=False)) as ds:
             assert equals_latlon(ds["temp"].attrs["coordinates"])
@@ -1241,7 +1244,7 @@
             }
         )
 
         with self.roundtrip(ds) as actual:
             # technically these datasets are slightly different,
             # one hold mixed endian data (ds) the other should be
             # all big endian (actual).  assertDatasetIdentical
@@ -1264,7 +1267,7 @@
         ):
             ds = Dataset({name: da})
             with pytest.raises(error) as excinfo:
                 with self.roundtrip(ds):
                     pass
             excinfo.match(msg)
             excinfo.match(repr(name))
@@ -1273,7 +1276,7 @@
         ds = Dataset({"x": ("y", np.arange(10.0))})
 
         kwargs: dict[str, Any] = dict(encoding={"x": {"dtype": "f4"}})
         with self.roundtrip(ds, save_kwargs=kwargs) as actual:
             encoded_dtype = actual.x.encoding["dtype"]
             # On OS X, dtype sometimes switches endianness for unclear reasons
             assert encoded_dtype.kind == "f" and encoded_dtype.itemsize == 4
@@ -1298,7 +1301,7 @@
         ds = Dataset({"t": pd.date_range("2000-01-01", periods=3)})
         units = "days since 1900-01-01"
         kwargs = dict(encoding={"t": {"units": units}})
         with self.roundtrip(ds, save_kwargs=kwargs) as actual:
             assert actual.t.encoding["units"] == units
             assert_identical(actual, ds)
 
@@ -1315,7 +1318,7 @@
         # Test default encoding for float:
         ds = Dataset({"x": ("y", np.arange(10.0))})
         kwargs = dict(encoding={"x": {"dtype": "f4"}})
         with self.roundtrip(ds, save_kwargs=kwargs) as actual:
             assert math.isnan(actual.x.encoding["_FillValue"])
         assert ds.x.encoding == {}
 
@@ -1337,34 +1340,34 @@
     def test_explicitly_omit_fill_value(self) -> None:
         ds = Dataset({"x": ("y", [np.pi, -np.pi])})
         ds.x.encoding["_FillValue"] = None
         with self.roundtrip(ds) as actual:
             assert "_FillValue" not in actual.x.encoding
 
     def test_explicitly_omit_fill_value_via_encoding_kwarg(self) -> None:
         ds = Dataset({"x": ("y", [np.pi, -np.pi])})
         kwargs = dict(encoding={"x": {"_FillValue": None}})
         # _FillValue is not a valid encoding for Zarr
         with self.roundtrip(ds, save_kwargs=kwargs) as actual:
             assert "_FillValue" not in actual.x.encoding
         assert ds.y.encoding == {}
 
     def test_explicitly_omit_fill_value_in_coord(self) -> None:
         ds = Dataset({"x": ("y", [np.pi, -np.pi])}, coords={"y": [0.0, 1.0]})
         ds.y.encoding["_FillValue"] = None
         with self.roundtrip(ds) as actual:
             assert "_FillValue" not in actual.y.encoding
 
     def test_explicitly_omit_fill_value_in_coord_via_encoding_kwarg(self) -> None:
         ds = Dataset({"x": ("y", [np.pi, -np.pi])}, coords={"y": [0.0, 1.0]})
         kwargs = dict(encoding={"y": {"_FillValue": None}})
         with self.roundtrip(ds, save_kwargs=kwargs) as actual:
             assert "_FillValue" not in actual.y.encoding
         assert ds.y.encoding == {}
 
     def test_encoding_same_dtype(self) -> None:
         ds = Dataset({"x": ("y", np.arange(10.0, dtype="f4"))})
         kwargs = dict(encoding={"x": {"dtype": "f4"}})
         with self.roundtrip(ds, save_kwargs=kwargs) as actual:
             encoded_dtype = actual.x.encoding["dtype"]
             # On OS X, dtype sometimes switches endianness for unclear reasons
             assert encoded_dtype.kind == "f" and encoded_dtype.itemsize == 4
@@ -1380,7 +1383,7 @@
         # regression for GH1215
         data = create_test_data()
         with create_tmp_file(allow_cleanup_failure=False) as tmp_file:
             self.save(data, tmp_file, mode="w")
             data["var2"][:] = -999
             data["var9"] = data["var2"] * 3
             self.save(data[["var2", "var9"]], tmp_file, mode="a")
@@ -2298,11 +2301,11 @@
             )
 
     def save(self, dataset, store_target, **kwargs):  # type: ignore[override]
         return dataset.to_zarr(store=store_target, **kwargs, **self.version_kwargs)
 
     @contextlib.contextmanager
     def open(self, path, **kwargs):
         with xr.open_dataset(
             path, engine="zarr", mode="r", **kwargs, **self.version_kwargs
         ) as ds:
             yield ds
@@ -2341,7 +2344,7 @@
                 RuntimeWarning,
                 match="Failed to open Zarr store with consolidated",
             ):
                 with xr.open_zarr(store, **self.version_kwargs) as ds:
                     assert_identical(ds, expected)
 
     def test_non_existent_store(self) -> None:
@@ -2443,7 +2446,7 @@
         good_chunks: tuple[dict[str, Any], ...] = ({"dim2": 3}, {"dim3": (6, 4)}, {})
         for chunks in good_chunks:
             kwargs = {"chunks": chunks}
             with assert_no_warnings():
                 with warnings.catch_warnings():
                     warnings.filterwarnings(
                         "ignore",
@@ -3008,7 +3011,7 @@
     @requires_dask
     def test_no_warning_from_open_emptydim_with_chunks(self) -> None:
         ds = Dataset({"x": (("a", "b"), np.empty((5, 0)))}).chunk({"a": 1})
         with assert_no_warnings():
             with warnings.catch_warnings():
                 warnings.filterwarnings(
                     "ignore",
@@ -3594,6 +3597,72 @@
             yield {}
 
 
+# TODO should we test that it doesn't warn if you explicitly pass a value for consolidate?
+@requires_zarr
+class TestDeprecateConsolidatedMetadataOnByDefault:
+    @pytest.fixture(autouse=True)
+    def create_empty_memorystore(self):
+        store = zarr.storage.MemoryStore({}, read_only=False)
+        # TODO I get a weird error if I don't create an empty root group, is that correct?
+        zarr.create_group(store=store)
+        # needed for open_dataarray to work
+        zarr.create_array(
+            store=store,
+            name="foo",
+            shape=(2,),
+            chunks=(2,),
+            dtype="int32",
+            dimension_names=["x"],
+        )
+        self.store = store
+
+    @pytest.mark.parametrize(
+        "open_func",
+        [
+            open_dataset,
+            open_dataarray,
+            open_datatree,
+            open_groups,
+            open_zarr,
+            load_dataarray,
+            load_dataset,
+        ],
+    )
+    def test_warn_on_open(self, open_func) -> None:
+        with pytest.warns(
+            PendingDeprecationWarning,
+            match="default value of the ``consolidated`` argument",
+        ):
+            if open_func is open_zarr:
+                open_zarr(self.store)
+            else:
+                open_func(self.store, engine="zarr")
+
+    def test_warn_on_dataarray_to_zarr(self, tmp_path) -> None:
+        da = xr.DataArray(1)
+        with pytest.warns(
+            PendingDeprecationWarning,
+            match="default value of the ``consolidated`` argument",
+        ):
+            da.to_zarr(tmp_path)
+
+    def test_warn_on_dataset_to_zarr(self, tmp_path) -> None:
+        ds = xr.Dataset()
+        with pytest.warns(
+            PendingDeprecationWarning,
+            match="default value of the ``consolidated`` argument",
+        ):
+            ds.to_zarr(tmp_path)
+
+    def test_warn_on_datatree_to_zarr(self, tmp_path) -> None:
+        dt = xr.DataTree(dataset=xr.Dataset({"a": 1}))
+        with pytest.warns(
+            PendingDeprecationWarning,
+            match="default value of the ``consolidated`` argument",
+        ):
+            dt.to_zarr(tmp_path)
+
+
 @requires_zarr
 @pytest.mark.skipif(
     ON_WINDOWS,
@@ -3755,10 +3824,10 @@
         store = {}
 
     with pytest.warns(FutureWarning, match="zarr_version"):
         ds.to_zarr(store=store, zarr_version=2)
 
     with pytest.warns(FutureWarning, match="zarr_version"):
         xr.open_zarr(store=store, zarr_version=2)
 
     with pytest.raises(ValueError, match="zarr_format"):
         xr.open_zarr(store=store, zarr_version=2, zarr_format=3)
@@ -5533,7 +5602,7 @@
         self.skip_if_zarr_python_3_and_zip_store(tmp_store)
         original_da = DataArray(np.arange(12).reshape((3, 4)))
 
         original_da.to_zarr(tmp_store)
 
         with open_dataarray(tmp_store, engine="zarr") as loaded_da:
             assert_identical(original_da, loaded_da)
@@ -5542,7 +5611,7 @@
         self.skip_if_zarr_python_3_and_zip_store(tmp_store)
         original_da = DataArray(np.arange(12).reshape((3, 4)), name="test")
 
         original_da.to_zarr(tmp_store)
 
         with open_dataarray(tmp_store, engine="zarr") as loaded_da:
             assert_identical(original_da, loaded_da)
@@ -5553,7 +5622,7 @@
             np.arange(12).reshape((3, 4)), dims=["x", "y"], name="x"
         )
 
         original_da.to_zarr(tmp_store)
 
         with open_dataarray(tmp_store, engine="zarr") as loaded_da:
             assert_identical(original_da, loaded_da)
@@ -5862,7 +5931,7 @@
 
     m = fsspec.filesystem("memory")
     mm = m.get_mapper("out1.zarr")
     ds.to_zarr(mm)  # old interface
     ds0 = ds.copy()
     # pd.to_timedelta returns ns-precision, but the example data is in second precision
     # so we need to fix this
@@ -5934,7 +6003,7 @@
         }
     )
     ds["test"].encoding["chunks"] = encoded_chunks
     ds.to_zarr(tmp_path / "test.zarr")
 
     with dask.config.set({"array.chunk-size": "1MiB"}):
         expected = ds.chunk(chunks)
@@ -6189,7 +6258,7 @@
             yield target, ds
 
     def save(self, target, ds, **kwargs):
         ds.to_zarr(target, **kwargs)
 
     @pytest.mark.parametrize(
         "region",