Merge pull request #2356 from jhamman/fix/consolidated-metadata-members

fix(consolidated metadata): skip .zmetadata key in members search

Author: jhamman

src/zarr/core/group.py

@@ -4,6 +4,7 @@
 import itertools
 import json
 import logging
+import warnings
 from collections import defaultdict
 from dataclasses import asdict, dataclass, field, fields, replace
 from typing import TYPE_CHECKING, Literal, TypeVar, assert_never, cast, overload
@@ -1139,7 +1140,8 @@ async def _members(
             raise ValueError(msg)
         # would be nice to make these special keys accessible programmatically,
         # and scoped to specific zarr versions
-        _skip_keys = ("zarr.json", ".zgroup", ".zattrs")
+        # especially true for `.zmetadata` which is configurable
+        _skip_keys = ("zarr.json", ".zgroup", ".zattrs", ".zmetadata")
 
         # hmm lots of I/O and logic interleaved here.
         # We *could* have an async gen over self.metadata.consolidated_metadata.metadata.keys()
@@ -1169,9 +1171,10 @@ async def _members(
                 # keyerror is raised when `key` names an object (in the object storage sense),
                 # as opposed to a prefix, in the store under the prefix associated with this group
                 # in which case `key` cannot be the name of a sub-array or sub-group.
-                logger.warning(
-                    "Object at %s is not recognized as a component of a Zarr hierarchy.",
-                    key,
+                warnings.warn(
+                    f"Object at {key} is not recognized as a component of a Zarr hierarchy.",
+                    UserWarning,
+                    stacklevel=1,
                 )
 
     def _members_consolidated(

tests/v3/test_group.py

@@ -1,6 +1,8 @@
 from __future__ import annotations
 
+import contextlib
 import pickle
+import warnings
 from typing import TYPE_CHECKING, Any, Literal, cast
 
 import numpy as np
@@ -177,22 +179,33 @@ def test_group_members(store: Store, zarr_format: ZarrFormat, consolidated_metad
         )
     )
 
+    # this warning shows up when extra objects show up in the hierarchy
+    warn_context = pytest.warns(
+        UserWarning, match=r"Object at .* is not recognized as a component of a Zarr hierarchy."
+    )
     if consolidated_metadata:
-        zarr.consolidate_metadata(store=store, zarr_format=zarr_format)
+        with warn_context:
+            zarr.consolidate_metadata(store=store, zarr_format=zarr_format)
+        # now that we've consolidated the store, we shouldn't get the warnings from the unrecognized objects anymore
+        # we use a nullcontext to handle these cases
+        warn_context = contextlib.nullcontext()
         group = zarr.open_consolidated(store=store, zarr_format=zarr_format)
 
-    members_observed = group.members()
+    with warn_context:
+        members_observed = group.members()
     # members are not guaranteed to be ordered, so sort before comparing
     assert sorted(dict(members_observed)) == sorted(members_expected)
 
     # partial
-    members_observed = group.members(max_depth=1)
+    with warn_context:
+        members_observed = group.members(max_depth=1)
     members_expected["subgroup/subsubgroup"] = subsubgroup
     # members are not guaranteed to be ordered, so sort before comparing
     assert sorted(dict(members_observed)) == sorted(members_expected)
 
     # total
-    members_observed = group.members(max_depth=None)
+    with warn_context:
+        members_observed = group.members(max_depth=None)
     members_expected["subgroup/subsubgroup/subsubsubgroup"] = subsubsubgroup
     # members are not guaranteed to be ordered, so sort before comparing
     assert sorted(dict(members_observed)) == sorted(members_expected)
@@ -1091,8 +1104,8 @@ async def test_require_array(store: Store, zarr_format: ZarrFormat) -> None:
 
 
 @pytest.mark.parametrize("consolidate", [True, False])
-def test_members_name(store: Store, consolidate: bool):
-    group = Group.from_store(store=store)
+async def test_members_name(store: Store, consolidate: bool, zarr_format: ZarrFormat):
+    group = Group.from_store(store=store, zarr_format=zarr_format)
     a = group.create_group(name="a")
     a.create_array("array", shape=(1,))
     b = a.create_group(name="b")
@@ -1108,6 +1121,12 @@ def test_members_name(store: Store, consolidate: bool):
     expected = ["/a", "/a/array", "/a/b", "/a/b/array"]
     assert paths == expected
 
+    # regression test for https://github.com/zarr-developers/zarr-python/pull/2356
+    g = zarr.open_group(store, use_consolidated=False)
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        assert list(g)
+
 
 async def test_open_mutable_mapping():
     group = await zarr.api.asynchronous.open_group(store={}, mode="w")