Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sdupport for r- read mode to read without consolidated metadata #183

Merged
merged 7 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
## 0.7.0 (Upcoming)
### Enhancements
* Added support for python 3.12. @mavaylon1 [#172](https://github.com/hdmf-dev/hdmf-zarr/pull/172)
* Added support for forcing read of files without consolidated metadata using `mode=r-` in `ZarrIO`. @oruebel [#183](https://github.com/hdmf-dev/hdmf-zarr/pull/183)

### Docs
* Removed `linkable` from the documentation to keep in line with `hdmf-schema-language`. @mavaylon1 [#180](https://github.com/hdmf-dev/hdmf-zarr/pull/180)

### Internal Fixes
* Fixed minor bug where `ZarrIO.__open_file_consolidated` used properties of `ZarrIO` instead of the provided input parameters. @oruebel [#183](https://github.com/hdmf-dev/hdmf-zarr/pull/183)

## 0.6.0 (February 21, 2024)

### Enhancements
Expand Down
23 changes: 16 additions & 7 deletions src/hdmf_zarr/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ def can_read(path):
'doc': 'the path to the Zarr file or a supported Zarr store'},
{'name': 'manager', 'type': BuildManager, 'doc': 'the BuildManager to use for I/O', 'default': None},
{'name': 'mode', 'type': str,
'doc': 'the mode to open the Zarr file with, one of ("w", "r", "r+", "a", "w-")'},
'doc': 'the mode to open the Zarr file with, one of ("w", "r", "r+", "a", "w-") '
'the mode r- is used to force open without consolidated metadata in read only mode.'},
{'name': 'synchronizer', 'type': (zarr.ProcessSynchronizer, zarr.ThreadSynchronizer, bool),
'doc': 'Zarr synchronizer to use for parallel I/O. If set to True a ProcessSynchronizer is used.',
'default': None},
Expand Down Expand Up @@ -160,8 +161,11 @@ def open(self):
# Within zarr, open_consolidated only allows the mode to be 'r' or 'r+'.
# As a result, when in other modes, the file will not use consolidated metadata.
if self.__mode not in ['r', 'r+']:
# r- is only an internal mode in ZarrIO to force the use of regular open. For Zarr we need to
# use the regular mode r when r- is specified
mode_to_use = self.__mode if self.__mode != 'r-' else 'r'
self.__file = zarr.open(store=self.path,
mode=self.__mode,
mode=mode_to_use,
synchronizer=self.__synchronizer,
storage_options=self.__storage_options)
else:
Expand Down Expand Up @@ -478,6 +482,9 @@ def __open_file_consolidated(self,
This method will check to see if the metadata has been consolidated.
If so, use open_consolidated.
"""
# This check is just a safeguard for possible errors in the future. But this should never happen
if mode == 'r-':
raise ValueError('Mode r- not allowed for reading with consolidated metadata')
# self.path can be both a string or a one of the `SUPPORTED_ZARR_STORES`.
if isinstance(self.path, str):
path = self.path
Expand All @@ -490,10 +497,10 @@ def __open_file_consolidated(self,
synchronizer=synchronizer,
storage_options=storage_options)
else:
return zarr.open(store=self.path,
mode=self.__mode,
synchronizer=self.__synchronizer,
storage_options=self.__storage_options)
return zarr.open(store=store,
mode=mode,
synchronizer=synchronizer,
storage_options=storage_options)

@docval({'name': 'parent', 'type': Group, 'doc': 'the parent Zarr object'},
{'name': 'builder', 'type': GroupBuilder, 'doc': 'the GroupBuilder to write'},
Expand Down Expand Up @@ -723,7 +730,9 @@ def resolve_ref(self, zarr_ref):
else:
target_name = ROOT_NAME

target_zarr_obj = self.__open_file_consolidated(source_file, mode='r', storage_options=self.__storage_options)
target_zarr_obj = self.__open_file_consolidated(store=source_file,
mode='r',
storage_options=self.__storage_options)
if object_path is not None:
try:
target_zarr_obj = target_zarr_obj[object_path]
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/base_tests_zarrio.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ def setUp(self):
self.store = "tests/unit/test_io.zarr"

def tearDown(self):
shutil.rmtree(self.store)
if os.path.exists(self.store):
shutil.rmtree(self.store)

def createReferenceBuilder(self):
data_1 = np.arange(100, 200, 10).reshape(2, 5)
Expand Down
30 changes: 30 additions & 0 deletions tests/unit/test_zarrio.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,33 @@ def test_get_store_path_deep(self):
path = ZarrIO._ZarrIO__get_store_path(store)
expected_path = os.path.normpath(os.path.join(CUR_DIR, 'test_io.zarr'))
self.assertEqual(path, expected_path)

def test_force_open_without_consolidated(self):
"""Test that read-mode -r forces a regular read with mode r"""
self.create_zarr(consolidate_metadata=True)
# Confirm that opening the file 'r' mode indeed uses the consolidated metadata
with ZarrIO(self.store, mode='r') as read_io:
read_io.open()
self.assertIsInstance(read_io.file.store, zarr.storage.ConsolidatedMetadataStore)
# Confirm that opening the file IN 'r-' mode indeed forces a regular open without consolidated metadata
with ZarrIO(self.store, mode='r-') as read_io:
read_io.open()
self.assertIsInstance(read_io.file.store, zarr.storage.DirectoryStore)

def test_force_open_without_consolidated_fails(self):
"""
Test that we indeed can't use '_ZarrIO__open_file_consolidated' function in r- read mode, which
is used to force read without consolidated metadata.
"""
self.create_zarr(consolidate_metadata=True)
with ZarrIO(self.store, mode='r') as read_io:
# Check that using 'r-' fails
msg = 'Mode r- not allowed for reading with consolidated metadata'
with self.assertRaisesWith(ValueError, msg):
read_io._ZarrIO__open_file_consolidated(store=self.store, mode='r-')
# Check that using 'r' does not fail
try:
read_io._ZarrIO__open_file_consolidated(store=self.store, mode='r')
except ValueError as e:
self.fail("ZarrIO.__open_file_consolidated raised an unexpected ValueError: {}".format(e))

Loading