diff --git a/zarr/__init__.py b/zarr/__init__.py index 7558ce77de..8a906534d1 100644 --- a/zarr/__init__.py +++ b/zarr/__init__.py @@ -11,7 +11,7 @@ from zarr.hierarchy import Group, group, open_group from zarr.n5 import N5Store, N5FSStore from zarr.storage import (ABSStore, DBMStore, DictStore, DirectoryStore, - LMDBStore, LRUStoreCache, MemoryStore, MongoDBStore, + KVStore, LMDBStore, LRUStoreCache, MemoryStore, MongoDBStore, NestedDirectoryStore, RedisStore, SQLiteStore, TempStore, ZipStore) from zarr.sync import ProcessSynchronizer, ThreadSynchronizer diff --git a/zarr/convenience.py b/zarr/convenience.py index 2cbc9bdf68..07d649a329 100644 --- a/zarr/convenience.py +++ b/zarr/convenience.py @@ -1277,7 +1277,9 @@ def open_consolidated(store: StoreLike, metadata_key=".zmetadata", mode="r+", ** """ # normalize parameters - store = normalize_store_arg(store, storage_options=kwargs.get("storage_options"), mode=mode) + zarr_version = kwargs.get('zarr_version', None) + store = normalize_store_arg(store, storage_options=kwargs.get("storage_options"), mode=mode, + zarr_version=zarr_version) if mode not in {'r', 'r+'}: raise ValueError("invalid mode, expected either 'r' or 'r+'; found {!r}" .format(mode)) diff --git a/zarr/hierarchy.py b/zarr/hierarchy.py index 0684be4a57..c2ddf451f1 100644 --- a/zarr/hierarchy.py +++ b/zarr/hierarchy.py @@ -1308,7 +1308,8 @@ def open_group(store=None, mode='a', cache_attrs=True, synchronizer=None, path=N if chunk_store is not None: chunk_store = _normalize_store_arg(chunk_store, storage_options=storage_options, - mode=mode) + mode=mode, + zarr_version=zarr_version) if not getattr(chunk_store, '_store_version', DEFAULT_ZARR_VERSION) == zarr_version: raise ValueError( "zarr_version of store and chunk_store must match" diff --git a/zarr/storage.py b/zarr/storage.py index 709bbba7ee..2a6b756d64 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -3045,7 +3045,20 @@ def rename(self, src_path: Path, dst_path: Path): src_parent, src_key = self._get_parent(base + src_path) dst_parent, dst_key = self._require_parent(base + dst_path) - dst_parent[dst_key] = src_parent.pop(src_key) + if src_key in src_parent: + dst_parent[dst_key] = src_parent.pop(src_key) + + if base == meta_root: + # check for and move corresponding metadata + sfx = _get_metadata_suffix(self) + src_meta = src_key + '.array' + sfx + if src_meta in src_parent: + dst_meta = dst_key + '.array' + sfx + dst_parent[dst_meta] = src_parent.pop(src_meta) + src_meta = src_key + '.group' + sfx + if src_meta in src_parent: + dst_meta = dst_key + '.group' + sfx + dst_parent[dst_meta] = src_parent.pop(src_meta) any_renamed = True any_renamed = _rename_metadata_v3(self, src_path, dst_path) or any_renamed if not any_renamed: diff --git a/zarr/tests/test_convenience.py b/zarr/tests/test_convenience.py index 74c8d06fac..53fa447b48 100644 --- a/zarr/tests/test_convenience.py +++ b/zarr/tests/test_convenience.py @@ -202,23 +202,30 @@ def test_tree(zarr_version): assert str(zarr.tree(g1)) == str(g1.tree()) -# TODO: consolidated metadata currently only supported for v2 - @pytest.mark.parametrize('zarr_version', [2, 3]) @pytest.mark.parametrize('with_chunk_store', [False, True], ids=['default', 'with_chunk_store']) -def test_consolidate_metadata(with_chunk_store, zarr_version): - - if zarr_version == 2: - MemoryStoreClass = MemoryStore - path = '' - else: - MemoryStoreClass = MemoryStoreV3 - path = 'dataset' - +@pytest.mark.parametrize('stores_from_path', [False, True]) +def test_consolidate_metadata(with_chunk_store, zarr_version, stores_from_path): # setup initial data - store = MemoryStoreClass() - chunk_store = MemoryStoreClass() if with_chunk_store else None - z = group(store, chunk_store=chunk_store, path=path) + if stores_from_path: + store = tempfile.mkdtemp() + atexit.register(atexit_rmtree, store) + if with_chunk_store: + chunk_store = tempfile.mkdtemp() + atexit.register(atexit_rmtree, chunk_store) + else: + chunk_store = None + version_kwarg = {'zarr_version': zarr_version} + else: + if zarr_version == 2: + store = MemoryStore() + chunk_store = MemoryStore() if with_chunk_store else None + elif zarr_version == 3: + store = MemoryStoreV3() + chunk_store = MemoryStoreV3() if with_chunk_store else None + version_kwarg = {} + path = 'dataset' if zarr_version == 3 else None + z = group(store, chunk_store=chunk_store, path=path, **version_kwarg) z.create_group('g1') g2 = z.create_group('g2') g2.attrs['hello'] = 'world' @@ -229,41 +236,48 @@ def test_consolidate_metadata(with_chunk_store, zarr_version): arr[:] = 1.0 assert 16 == arr.nchunks_initialized + if stores_from_path: + # get the actual store class for use with consolidate_metadata + store_class = z._store + else: + store_class = store + if zarr_version == 3: # error on v3 if path not provided with pytest.raises(ValueError): - consolidate_metadata(store, path=None) + consolidate_metadata(store_class, path=None) with pytest.raises(ValueError): - consolidate_metadata(store, path='') + consolidate_metadata(store_class, path='') # perform consolidation - out = consolidate_metadata(store, path=path) + out = consolidate_metadata(store_class, path=path) assert isinstance(out, Group) assert ['g1', 'g2'] == list(out) - if zarr_version == 2: - assert isinstance(out._store, ConsolidatedMetadataStore) - assert '.zmetadata' in store - meta_keys = ['.zgroup', - 'g1/.zgroup', - 'g2/.zgroup', - 'g2/.zattrs', - 'g2/arr/.zarray', - 'g2/arr/.zattrs'] - else: - assert isinstance(out._store, ConsolidatedMetadataStoreV3) - assert 'meta/root/consolidated/.zmetadata' in store - meta_keys = ['zarr.json', - meta_root + 'dataset.group.json', - meta_root + 'dataset/g1.group.json', - meta_root + 'dataset/g2.group.json', - meta_root + 'dataset/g2/arr.array.json', - 'meta/root/consolidated.group.json'] - for key in meta_keys: - del store[key] + if not stores_from_path: + if zarr_version == 2: + assert isinstance(out._store, ConsolidatedMetadataStore) + assert '.zmetadata' in store + meta_keys = ['.zgroup', + 'g1/.zgroup', + 'g2/.zgroup', + 'g2/.zattrs', + 'g2/arr/.zarray', + 'g2/arr/.zattrs'] + else: + assert isinstance(out._store, ConsolidatedMetadataStoreV3) + assert 'meta/root/consolidated/.zmetadata' in store + meta_keys = ['zarr.json', + meta_root + 'dataset.group.json', + meta_root + 'dataset/g1.group.json', + meta_root + 'dataset/g2.group.json', + meta_root + 'dataset/g2/arr.array.json', + 'meta/root/consolidated.group.json'] + for key in meta_keys: + del store[key] # open consolidated - z2 = open_consolidated(store, chunk_store=chunk_store, path=path) + z2 = open_consolidated(store, chunk_store=chunk_store, path=path, **version_kwarg) assert ['g1', 'g2'] == list(z2) assert 'world' == z2.g2.attrs['hello'] assert 1 == z2.g2.arr.attrs['data'] @@ -271,22 +285,32 @@ def test_consolidate_metadata(with_chunk_store, zarr_version): assert 16 == z2.g2.arr.nchunks assert 16 == z2.g2.arr.nchunks_initialized - # tests del/write on the store - if zarr_version == 2: - cmd = ConsolidatedMetadataStore(store) - with pytest.raises(PermissionError): - del cmd['.zgroup'] - with pytest.raises(PermissionError): - cmd['.zgroup'] = None + if stores_from_path: + # path string is note a BaseStore subclass so cannot be used to + # initialize a ConsolidatedMetadataStore. + if zarr_version == 2: + with pytest.raises(ValueError): + cmd = ConsolidatedMetadataStore(store) + elif zarr_version == 3: + with pytest.raises(ValueError): + cmd = ConsolidatedMetadataStoreV3(store) else: - cmd = ConsolidatedMetadataStoreV3(store) - with pytest.raises(PermissionError): - del cmd[meta_root + 'dataset.group.json'] - with pytest.raises(PermissionError): - cmd[meta_root + 'dataset.group.json'] = None + # tests del/write on the store + if zarr_version == 2: + cmd = ConsolidatedMetadataStore(store) + with pytest.raises(PermissionError): + del cmd['.zgroup'] + with pytest.raises(PermissionError): + cmd['.zgroup'] = None + else: + cmd = ConsolidatedMetadataStoreV3(store) + with pytest.raises(PermissionError): + del cmd[meta_root + 'dataset.group.json'] + with pytest.raises(PermissionError): + cmd[meta_root + 'dataset.group.json'] = None - # test getsize on the store - assert isinstance(getsize(cmd), Integral) + # test getsize on the store + assert isinstance(getsize(cmd), Integral) # test new metadata are not writeable with pytest.raises(PermissionError): @@ -316,62 +340,11 @@ def test_consolidate_metadata(with_chunk_store, zarr_version): # make sure keyword arguments are passed through without error open_consolidated( - store, chunk_store=chunk_store, path=path, cache_attrs=True, synchronizer=None + store, chunk_store=chunk_store, path=path, cache_attrs=True, synchronizer=None, + **version_kwarg, ) -def test_consolidated_with_chunk_store(): - # setup initial data - store = MemoryStore() - chunk_store = MemoryStore() - z = group(store, chunk_store=chunk_store) - z.create_group('g1') - g2 = z.create_group('g2') - g2.attrs['hello'] = 'world' - arr = g2.create_dataset('arr', shape=(20, 20), chunks=(5, 5), dtype='f8') - assert 16 == arr.nchunks - assert 0 == arr.nchunks_initialized - arr.attrs['data'] = 1 - arr[:] = 1.0 - assert 16 == arr.nchunks_initialized - - # perform consolidation - out = consolidate_metadata(store) - assert isinstance(out, Group) - assert '.zmetadata' in store - for key in ['.zgroup', - 'g1/.zgroup', - 'g2/.zgroup', - 'g2/.zattrs', - 'g2/arr/.zarray', - 'g2/arr/.zattrs']: - del store[key] - # open consolidated - z2 = open_consolidated(store, chunk_store=chunk_store) - assert ['g1', 'g2'] == list(z2) - assert 'world' == z2.g2.attrs['hello'] - assert 1 == z2.g2.arr.attrs['data'] - assert (z2.g2.arr[:] == 1.0).all() - assert 16 == z2.g2.arr.nchunks - assert 16 == z2.g2.arr.nchunks_initialized - - # test the data are writeable - z2.g2.arr[:] = 2 - assert (z2.g2.arr[:] == 2).all() - - # test invalid modes - with pytest.raises(ValueError): - open_consolidated(store, mode='a', chunk_store=chunk_store) - with pytest.raises(ValueError): - open_consolidated(store, mode='w', chunk_store=chunk_store) - with pytest.raises(ValueError): - open_consolidated(store, mode='w-', chunk_store=chunk_store) - - # make sure keyword arguments are passed through without error - open_consolidated(store, cache_attrs=True, synchronizer=None, - chunk_store=chunk_store) - - @pytest.mark.parametrize("options", ( {"dimension_separator": "/"}, {"dimension_separator": "."}, diff --git a/zarr/tests/test_core.py b/zarr/tests/test_core.py index 08bda94ba2..09523dcd22 100644 --- a/zarr/tests/test_core.py +++ b/zarr/tests/test_core.py @@ -2720,8 +2720,8 @@ def test_array_init(self): assert isinstance(a, Array) assert (100,) == a.shape assert (10,) == a.chunks - assert path == a.path # TODO: should this include meta/root? - assert '/' + path == a.name # TODO: should this include meta/root? + assert path == a.path + assert '/' + path == a.name assert 'bar' == a.basename assert store is a.store assert "968dccbbfc0139f703ead2fd1d503ad6e44db307" == a.hexdigest() @@ -2772,7 +2772,7 @@ def test_nbytes_stored(self): z[:] = 42 expect_nbytes_stored = sum(buffer_size(v) for k, v in z.store.items() if k != 'zarr.json') assert expect_nbytes_stored == z.nbytes_stored - assert z.nchunks_initialized == 10 # TODO: added temporarily for testing, can remove + assert z.nchunks_initialized == 10 # mess with store if not isinstance(z.store, (LRUStoreCacheV3, FSStoreV3)): diff --git a/zarr/tests/test_hierarchy.py b/zarr/tests/test_hierarchy.py index 69ab08254e..29fff7536a 100644 --- a/zarr/tests/test_hierarchy.py +++ b/zarr/tests/test_hierarchy.py @@ -28,7 +28,7 @@ NestedDirectoryStore, SQLiteStore, ZipStore, array_meta_key, atexit_rmglob, atexit_rmtree, data_root, group_meta_key, init_array, init_group, meta_root) -from zarr.storage import (ABSStoreV3, KVStoreV3, DirectoryStoreV3, # MemoryStoreV3 +from zarr.storage import (ABSStoreV3, KVStoreV3, DirectoryStoreV3, MemoryStoreV3, FSStoreV3, ZipStoreV3, DBMStoreV3, LMDBStoreV3, SQLiteStoreV3, LRUStoreCacheV3) from zarr.util import InfoReporter, buffer_size @@ -233,8 +233,14 @@ def test_require_group(self): # test path normalization if g1._version == 2: - # TODO: expected behavior for v3 assert g1.require_group('quux') == g1.require_group('/quux/') + elif g1._version: + # These are not equal in v3! + # 'quux' will be within the group: + # meta/root/group/quux.group.json + # '/quux/' will be outside of the group at: + # meta/root/quux.group.json + assert g1.require_group('quux') != g1.require_group('/quux/') # multi g6, g7 = g1.require_groups('y', 'z') @@ -519,7 +525,7 @@ def test_getitem_contains_iterators(self): assert isinstance(g1['/foo/bar/'], Group) else: # start or end with / raises KeyError - # TODO: should we fix allow stripping of these on v3? + # TODO: should we allow stripping of these on v3? with pytest.raises(KeyError): assert isinstance(g1['/foo/bar/'], Group) assert isinstance(g1['foo/baz'], Array) @@ -547,9 +553,7 @@ def test_getitem_contains_iterators(self): assert 'baz' not in g1 assert 'a/b/c/d' not in g1 assert 'a/z' not in g1 - if g1._version == 2: - # TODO: handle implicit group for v3 spec - assert 'quux' not in g1['foo'] + assert 'quux' not in g1['foo'] # test key errors with pytest.raises(KeyError): @@ -888,12 +892,27 @@ def test_move(self): assert "foo2" in g assert "foo2/bar" not in g if g2._version == 2: - # TODO: how to access element created outside of group.path in v3? assert "bar" in g + else: + # The `g2.move` call above moved bar to meta/root/bar and + # meta/data/bar. This is outside the `g` group located at + # /meta/root/group, so bar is no longer within `g`. + assert "bar" not in g + assert 'meta/root/bar.array.json' in g._store + if g._chunk_store: + assert 'data/root/bar/c0' in g._chunk_store + else: + assert 'data/root/bar/c0' in g._store assert isinstance(g["foo2"], Group) if g2._version == 2: - # TODO: how to access element created outside of group.path in v3? assert_array_equal(data, g["bar"]) + else: + # TODO: How to access element created outside of group.path in v3? + # One option is to make a Hierarchy class representing the + # root. Currently Group requires specification of `path`, + # but the path of the root would be just '' which is not + # currently allowed. + pass with pytest.raises(ValueError): g2.move("bar", "bar2") @@ -970,22 +989,44 @@ def test_paths(self): g1 = self.create_group() g2 = g1.create_group('foo/bar') - if g1._version == 3: - pytest.skip("TODO: update class for v3") - - assert g1 == g1['/'] - assert g1 == g1['//'] - assert g1 == g1['///'] - assert g1 == g2['/'] - assert g1 == g2['//'] - assert g1 == g2['///'] - assert g2 == g1['foo/bar'] - assert g2 == g1['/foo/bar'] - assert g2 == g1['foo/bar/'] - assert g2 == g1['//foo/bar'] - assert g2 == g1['//foo//bar//'] - assert g2 == g1['///foo///bar///'] - assert g2 == g2['/foo/bar'] + if g1._version == 2: + assert g1 == g1['/'] + assert g1 == g1['//'] + assert g1 == g1['///'] + assert g1 == g2['/'] + assert g1 == g2['//'] + assert g1 == g2['///'] + assert g2 == g1['foo/bar'] + assert g2 == g1['/foo/bar'] + assert g2 == g1['foo/bar/'] + assert g2 == g1['//foo/bar'] + assert g2 == g1['//foo//bar//'] + assert g2 == g1['///foo///bar///'] + assert g2 == g2['/foo/bar'] + else: + # the expected key format gives a match + assert g2 == g1['foo/bar'] + + # TODO: Should presence of a trailing slash raise KeyError? + # The spec says "the final character is not a / character" + # but we currently strip trailing '/' as done for v2. + assert g2 == g1['foo/bar/'] + + # double slash also currently works (spec doesn't mention this + # case, but have kept it for v2 behavior compatibility) + assert g2 == g1['foo//bar'] + + # v3: leading / implies we are at the root, not within a group, + # so these all raise KeyError + for path in ['/foo/bar', '//foo/bar', '//foo//bar//', + '///fooo///bar///']: + with pytest.raises(KeyError): + g1[path] + + # For v3 a prefix must be supplied + for path in ['/', '//', '///']: + with pytest.raises(ValueError): + g2[path] with pytest.raises(ValueError): g1['.'] @@ -1025,9 +1066,7 @@ def test_pickle(self): assert name == g2.name assert n == len(g2) assert keys == list(g2) - if g2._version == 2: - # TODO: handle implicit group for v3 - assert isinstance(g2['foo'], Group) + assert isinstance(g2['foo'], Group) assert isinstance(g2['foo/bar'], Array) g2.store.close() @@ -1113,13 +1152,13 @@ def create_store(): return MemoryStore(), None -# TODO: fix MemoryStoreV3 _get_parent, etc. -# # noinspection PyStatementEffect -# class TestGroupV3WithMemoryStore(TestGroupWithMemoryStore, TestGroupV3): +# noinspection PyStatementEffect +class TestGroupV3WithMemoryStore(TestGroupWithMemoryStore, TestGroupV3): + + @staticmethod + def create_store(): + return MemoryStoreV3(), None -# @staticmethod -# def create_store(): -# return MemoryStoreV3(), None class TestGroupWithDirectoryStore(TestGroup): @@ -1158,7 +1197,7 @@ def test_pickle(self): @skip_test_env_var("ZARR_TEST_ABS") -class TestGroupWithABSStoreV3(TestGroupV3): +class TestGroupV3WithABSStore(TestGroupV3): @staticmethod def create_store(): @@ -1550,9 +1589,6 @@ def test_open_group(zarr_version): g.create_groups('foo', 'bar') assert 2 == len(g) - # TODO: update the r, r+ test case here for zarr_version == 3 after - # open_array has StoreV3 support - # mode in 'r', 'r+' open_array('data/array.zarr', shape=100, chunks=10, mode='w') for mode in 'r', 'r+': @@ -1816,3 +1852,15 @@ def test_group_mismatched_store_versions(): Group(store_v3, path='group2', read_only=True, chunk_store=chunk_store_v3) with pytest.raises(ValueError): Group(store_v3, path='group2', read_only=True, chunk_store=chunk_store_v3) + + +@pytest.mark.parametrize('zarr_version', [2, 3]) +def test_open_group_from_paths(zarr_version): + """Verify zarr_version is applied to both the store and chunk_store.""" + store = tempfile.mkdtemp() + chunk_store = tempfile.mkdtemp() + atexit.register(atexit_rmtree, store) + atexit.register(atexit_rmtree, chunk_store) + path = 'g1' + g = open_group(store, path=path, chunk_store=chunk_store, zarr_version=zarr_version) + assert g._store._store_version == g._chunk_store._store_version == zarr_version