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

…183)

* Add sdupport for r- read mode to read without consolidated metadata
* Updated CHANGELOG for #183
* Add error check and fix bug in ZarrIO.__open_consolidate
* Avoid error on clean-up if the file was not created in a test
* Add unit tests for r- read mode

CHANGELOG.md

@@ -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

src/hdmf_zarr/backend.py

@@ -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},
@@ -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:
@@ -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
@@ -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'},
@@ -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]

tests/unit/base_tests_zarrio.py

@@ -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)

tests/unit/test_zarrio.py

@@ -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))
+