Skip to content

Commit

Permalink
feedback fix
Browse files Browse the repository at this point in the history
  • Loading branch information
mavaylon1 committed Sep 29, 2023
1 parent 67db293 commit 1a4014d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 26 deletions.
12 changes: 5 additions & 7 deletions docs/source/storage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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:

Expand Down Expand Up @@ -381,7 +383,3 @@ data type. The specification of the namespace is stored in
``/specifications/<namespace-name>/<version>/<source-filename>``. Here ``<source-filename>`` 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``).




26 changes: 13 additions & 13 deletions src/hdmf_zarr/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
9 changes: 3 additions & 6 deletions tests/unit/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from hdmf.utils import (docval, getargs, get_docval)

CORE_NAMESPACE = 'test_core'
ROOT_NAME = 'root'


class CacheSpecTestHelper(object):
Expand Down Expand Up @@ -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},
Expand All @@ -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
Expand Down Expand Up @@ -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})
Expand Down

0 comments on commit 1a4014d

Please sign in to comment.