From 1a4014d0fb7ca521e3184fe30d795141aca02c20 Mon Sep 17 00:00:00 2001 From: mavaylon1 Date: Fri, 29 Sep 2023 15:13:02 -0700 Subject: [PATCH] feedback fix --- docs/source/storage.rst | 12 +++++------- src/hdmf_zarr/backend.py | 26 +++++++++++++------------- tests/unit/utils.py | 9 +++------ 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/docs/source/storage.rst b/docs/source/storage.rst index 281f7b5f..1cb98576 100644 --- a/docs/source/storage.rst +++ b/docs/source/storage.rst @@ -282,8 +282,6 @@ region references link to subsets of another Dataset. To identify region referen to the ``source`` and ``path``, the py:class:`~hdmf_zarr.utils.ZarrReference` object will also need to store the definition of the ``region`` that is being referenced, e.g., a slice or list on point indices. -In the future, the reference resolver classes will need to be updated to implement region references - .. warning:: Region references are not yet fully implemented in :py:class:`~hdmf_zarr.backend.ZarrIO`. @@ -298,7 +296,11 @@ In the future, the reference resolver classes will need to be updated to impleme 4) :py:meth:`~hdmf_zarr.backend.ZarrIO.__read_dataset` to support reading region references, which may also require updates to :py:meth:`~hdmf_zarr.backend.ZarrIO.__parse_ref` and :py:meth:`~hdmf_zarr.backend.ZarrIO.__resolve_ref`, and - 5) and possibly other parts of :py:class:`~hdmf_zarr.backend.ZarrIO` + 5) and possibly other parts of :py:class:`~hdmf_zarr.backend.ZarrIO`. + 6) The py:class:`~hdmf_zarr.zarr_utils.ContainerZarrRegionDataset` and + py:class:`~hdmf_zarr.zarr_utils.ContainerZarrRegionDataset` classes will also need to be finalized + to support region references. + .. _sec-zarr-storage-dtypes: @@ -381,7 +383,3 @@ data type. The specification of the namespace is stored in ``/specifications///``. Here ```` refers to the main name of the source-file without file extension (e.g., the core namespace defines ``nwb.ephys.yaml`` as source which would be stored in ``/specifications/core/2.0.1/nwb.ecephys``). - - - - diff --git a/src/hdmf_zarr/backend.py b/src/hdmf_zarr/backend.py index 4f588c23..0b5a8f1a 100644 --- a/src/hdmf_zarr/backend.py +++ b/src/hdmf_zarr/backend.py @@ -1068,6 +1068,19 @@ def __set_built(self, zarr_obj, builder): path = os.path.join(fpath, path) self.__built.setdefault(path, builder) + @docval({'name': 'zarr_obj', 'type': (Array, Group), + 'doc': 'the Zarr object to the corresponding Container/Data object for'}) + def get_container(self, **kwargs): + """ + Get the container for the corresponding Zarr Group or Dataset + + :raises ValueError: When no builder has been constructed yet for the given h5py object + """ + zarr_obj = getargs('zarr_obj', kwargs) + builder = self.get_builder(zarr_obj) + container = self.manager.construct(builder) + return container # TODO: This method should be moved to HDMFIO + @docval({'name': 'zarr_obj', 'type': (Array, Group), 'doc': 'the Zarr object to the corresponding Builder object for'}) def get_builder(self, **kwargs): # TODO: move this to HDMFIO (define skeleton in there at least) @@ -1226,16 +1239,3 @@ def __read_attrs(self, zarr_obj): else: ret[k] = v return ret - - @docval({'name': 'zarr_obj', 'type': (Array, Group), - 'doc': 'the Zarr object to the corresponding Container/Data object for'}) - def get_container(self, **kwargs): - """ - Get the container for the corresponding Zarr Group or Dataset - - :raises ValueError: When no builder has been constructed yet for the given h5py object - """ - zarr_obj = getargs('zarr_obj', kwargs) - builder = self.get_builder(zarr_obj) - container = self.manager.construct(builder) - return container # TODO: This method should be moved to HDMFIO diff --git a/tests/unit/utils.py b/tests/unit/utils.py index 123536e0..1fb3d5be 100644 --- a/tests/unit/utils.py +++ b/tests/unit/utils.py @@ -11,6 +11,7 @@ from hdmf.utils import (docval, getargs, get_docval) CORE_NAMESPACE = 'test_core' +ROOT_NAME = 'root' class CacheSpecTestHelper(object): @@ -113,8 +114,6 @@ class FooFile(Container): and should be reset to 'root' when use is finished to avoid potential cross-talk between tests. """ - ROOT_NAME = 'root' # For HDF5 and Zarr this is the root. It should be set before use if different for the backend. - @docval({'name': 'buckets', 'type': list, 'doc': 'the FooBuckets in this file', 'default': list()}, {'name': 'foo_link', 'type': Foo, 'doc': 'an optional linked Foo', 'default': None}, {'name': 'foofile_data', 'type': 'array_data', 'doc': 'an optional dataset', 'default': None}, @@ -123,7 +122,7 @@ class FooFile(Container): def __init__(self, **kwargs): buckets, foo_link, foofile_data, foo_ref_attr = getargs('buckets', 'foo_link', 'foofile_data', 'foo_ref_attr', kwargs) - super().__init__(name=self.ROOT_NAME) # name is not used - FooFile should be the root container + super().__init__(name=ROOT_NAME) # name is not used - FooFile should be the root container self.__buckets = {b.name: b for b in buckets} # note: collections of groups are unordered in HDF5 for f in buckets: f.parent = self @@ -306,9 +305,7 @@ class BazCpdData(Data): class BazBucket(Container): - ROOT_NAME = 'root' - - @docval({'name': 'name', 'type': str, 'doc': 'the name of this bucket', 'default': 'root'}, + @docval({'name': 'name', 'type': str, 'doc': 'the name of this bucket', 'default': ROOT_NAME}, {'name': 'bazs', 'type': list, 'doc': 'the Baz objects in this bucket'}, {'name': 'baz_data', 'type': BazData, 'doc': 'dataset of Baz references', 'default': None}, {'name': 'baz_cpd_data', 'type': BazCpdData, 'doc': 'dataset of Baz references', 'default': None})