Do not expose open() and close() to user in dataset api (#448)

* do not expose open() and close() to user

* update changelog

* implement PR feedback

* format code

* fix comments

* fix code after merging master

* remove import

Author: rschwanhold

webknossos/Changelog.md

@@ -18,6 +18,7 @@ For upgrade instructions, please check the respective *Breaking Changes* section
 - Added dataset down- and upload as well as annotation download, see the examples `learned_segmenter.py` and `upload_image_data.py`. [#437](https://github.com/scalableminds/webknossos-libs/pull/452)
 
 ### Changed
+- `View`s now always open the `wkw.Dataset` lazily. All explicit calls to `View.open()` and `View.close()` must be removed. [#448](https://github.com/scalableminds/webknossos-libs/pull/448)
 ### Fixed
 
 

webknossos/tests/test_dataset.py

@@ -9,7 +9,6 @@
 
 import numpy as np
 import pytest
-from wkw.wkw import WKWException
 
 from webknossos.dataset import (
     Dataset,
@@ -223,41 +222,18 @@ def test_modify_existing_dataset() -> None:
     assure_exported_properties(ds2)
 
 
-def test_view_read_with_open() -> None:
+def test_view_read() -> None:
     wk_view = (
         Dataset(TESTDATA_DIR / "simple_wk_dataset")
         .get_layer("color")
         .get_mag("1")
         .get_view(size=(16, 16, 16))
     )
 
-    assert not wk_view._is_opened
-
-    with wk_view.open():
-        assert wk_view._is_opened
-
-        data = wk_view.read(size=(10, 10, 10))
-        assert data.shape == (3, 10, 10, 10)  # three channel
-
-    assert not wk_view._is_opened
-
-
-def test_view_read_without_open() -> None:
-    wk_view = (
-        Dataset(TESTDATA_DIR / "simple_wk_dataset")
-        .get_layer("color")
-        .get_mag("1")
-        .get_view(size=(16, 16, 16))
-    )
-
-    assert not wk_view._is_opened
-
-    # 'read()' checks if it was already opened. If not, it opens and closes automatically
+    # 'read()' checks if it was already opened. If not, it opens it automatically
     data = wk_view.read(size=(10, 10, 10))
     assert data.shape == (3, 10, 10, 10)  # three channel
 
-    assert not wk_view._is_opened
-
 
 def test_view_write() -> None:
     delete_dir(TESTOUTPUT_DIR / "simple_wk_dataset")
@@ -270,14 +246,13 @@ def test_view_write() -> None:
         .get_view(size=(16, 16, 16))
     )
 
-    with wk_view.open():
-        np.random.seed(1234)
-        write_data = (np.random.rand(3, 10, 10, 10) * 255).astype(np.uint8)
+    np.random.seed(1234)
+    write_data = (np.random.rand(3, 10, 10, 10) * 255).astype(np.uint8)
 
-        wk_view.write(write_data)
+    wk_view.write(write_data)
 
-        data = wk_view.read(size=(10, 10, 10))
-        assert np.array_equal(data, write_data)
+    data = wk_view.read(size=(10, 10, 10))
+    assert np.array_equal(data, write_data)
 
 
 def test_view_write_out_of_bounds() -> None:
@@ -293,11 +268,10 @@ def test_view_write_out_of_bounds() -> None:
         .get_view(size=(16, 16, 16))
     )
 
-    with view.open():
-        with pytest.raises(AssertionError):
-            view.write(
-                np.zeros((200, 200, 5), dtype=np.uint8)
-            )  # this is bigger than the bounding_box
+    with pytest.raises(AssertionError):
+        view.write(
+            np.zeros((200, 200, 5), dtype=np.uint8)
+        )  # this is bigger than the bounding_box
 
 
 def test_mag_view_write_out_of_bounds() -> None:
@@ -686,29 +660,6 @@ def test_chunking_wk_wrong_chunk_size() -> None:
     assure_exported_properties(ds)
 
 
-def test_view_write_without_open() -> None:
-    ds_path = TESTOUTPUT_DIR / "wk_dataset_write_without_open"
-    delete_dir(ds_path)
-
-    ds = Dataset.create(ds_path, scale=(1, 1, 1))
-    layer = ds.add_layer("color", LayerCategories.COLOR_TYPE)
-    layer.bounding_box = BoundingBox(
-        (0, 0, 0), (64, 64, 64)
-    )  # This newly created dataset would otherwise have a "empty" bounding box
-    mag = layer.add_mag("1")
-
-    wk_view = mag.get_view(size=(32, 64, 16))
-
-    assert not wk_view._is_opened
-
-    write_data = (np.random.rand(32, 64, 16) * 255).astype(np.uint8)
-    wk_view.write(write_data)
-
-    assert not wk_view._is_opened
-
-    assure_exported_properties(ds)
-
-
 def test_typing_of_get_mag() -> None:
     ds = Dataset(TESTDATA_DIR / "simple_wk_dataset")
     layer = ds.get_layer("color")
@@ -1340,9 +1291,8 @@ def test_search_dataset_also_for_long_layer_name() -> None:
     # rename the path from "long_layer_name/color/2" to "long_layer_name/color/2-2-2"
     os.rename(short_mag_file_path, long_mag_file_path)
 
-    with pytest.raises(WKWException):
-        # the dataset has to be reopened to notice the changed directory
-        mag.read(offset=(10, 10, 10), size=(10, 10, 10))
+    # make sure that reading data still works
+    mag.read(offset=(10, 10, 10), size=(10, 10, 10))
 
     # when opening the dataset, it searches both for the long and the short path
     layer = Dataset(TESTOUTPUT_DIR / "long_layer_name").get_layer("color")

webknossos/webknossos/dataset/downsampling_utils.py

@@ -307,8 +307,6 @@ def downsample_cube_job(
             )
         )
 
-        source_view.open()
-
         for tile in tiles:
             target_offset = np.array(tile) * buffer_edge_len
             source_offset = (mag_factors * target_offset).astype(int)
@@ -348,7 +346,6 @@ def downsample_cube_job(
                         buffer_offset[2] : buffer_end[2],
                     ] = data_cube
 
-        source_view.close()
         # Write the downsampled buffer to target
         if source_view.header.num_channels == 1:
             file_buffer = file_buffer[0]  # remove channel dimension

webknossos/webknossos/dataset/layer.py

@@ -217,10 +217,9 @@ def name(self, layer_name: str) -> None:
         # The MagViews need to be updated
         for mag in self._mags.values():
             mag._path = _find_mag_path_on_disk(self.dataset.path, self.name, mag.name)
-            if mag._is_opened:
-                # Reopen handle to dataset on disk
-                mag.close()
-                mag.open()
+            # Deleting the dataset will close the file handle.
+            # The new dataset will be opened automatically when needed.
+            del mag._wkw_dataset
 
         self.dataset._export_as_json()
 

webknossos/webknossos/dataset/mag_view.py

@@ -244,22 +244,13 @@ def get_bounding_boxes_on_disk(
         - the bounding box in the properties is an "overall" bounding box, which abstracts from the files on disk
         """
         cube_size = self._get_file_dimensions()
-        was_opened = self._is_opened
-
-        if not was_opened:
-            self.open()  # opening the view is necessary to set the dataset
-
-        assert self._dataset is not None
-        for filename in self._dataset.list_files():
+        for filename in self._wkw_dataset.list_files():
             file_path = Path(os.path.splitext(filename)[0]).relative_to(self._path)
             cube_index = _extract_file_index(file_path)
             cube_offset = [idx * size for idx, size in zip(cube_index, cube_size)]
 
             yield (cube_offset[0], cube_offset[1], cube_offset[2]), cube_size
 
-        if not was_opened:
-            self.close()
-
     def compress(
         self,
         target_path: Optional[Union[str, Path]] = None,
@@ -296,19 +287,13 @@ def compress(
                 self.name, str(uncompressed_full_path)
             )
         )
-
-        was_opened = self._is_opened
-        if not was_opened:
-            self.open()  # opening the view is necessary to set the dataset
-        assert self._dataset is not None
-
         # create empty wkw.Dataset
-        self._dataset.compress(str(compressed_full_path))
+        self._wkw_dataset.compress(str(compressed_full_path))
 
         # compress all files to and move them to 'compressed_path'
         with get_executor_for_args(args) as executor:
             job_args = []
-            for file in self._dataset.list_files():
+            for file in self._wkw_dataset.list_files():
                 rel_file = Path(file).relative_to(self.layer.dataset.path)
                 job_args.append((Path(file), compressed_path / rel_file))
 
@@ -318,9 +303,6 @@ def compress(
 
         logging.info("Mag {0} successfully compressed".format(self.name))
 
-        if not was_opened:
-            self.close()
-
         if target_path is None:
             shutil.rmtree(uncompressed_full_path)
             shutil.move(str(compressed_full_path), uncompressed_full_path)

webknossos/webknossos/dataset/upsampling_utils.py

@@ -57,8 +57,6 @@ def upsample_cube_job(
             )
         )
 
-        source_view.open()
-
         for tile in tiles:
             target_offset = np.array(tile) * buffer_edge_len
             source_offset = (mag_factors * target_offset).astype(int)
@@ -95,7 +93,6 @@ def upsample_cube_job(
                         buffer_offset[2] : buffer_end[2],
                     ] = data_cube
 
-        source_view.close()
         # Write the upsampled buffer to target
         if source_view.header.num_channels == 1:
             file_buffer = file_buffer[0]  # remove channel dimension

webknossos/webknossos/dataset/view.py

@@ -38,14 +38,13 @@ def __init__(
         """
         Do not use this constructor manually. Instead use `webknossos.dataset.mag_view.MagView.get_view()` to get a `View`.
         """
-        self._dataset: Optional[Dataset] = None
         self._path = path_to_mag_view
         self._header: wkw.Header = header
         self._size: Vec3Int = Vec3Int(size)
         self._global_offset: Vec3Int = Vec3Int(global_offset)
         self._is_bounded = is_bounded
         self._read_only = read_only
-        self._is_opened = False
+        self._cached_wkw_dataset = None
         # The bounding box of the view is used to prevent warnings when writing compressed but unaligned data
         # directly at the borders of the bounding box.
         # A View is unable to get this information from the Dataset because it is detached from it.
@@ -73,37 +72,6 @@ def global_offset(self) -> Vec3Int:
     def read_only(self) -> bool:
         return self._read_only
 
-    def open(self) -> "View":
-        """
-        Opens the actual handles to the data on disk.
-        A `MagDataset` has to be opened before it can be read or written to. However, the user does not
-        have to open it explicitly because the API automatically opens it when it is needed.
-        The user can choose to open it explicitly to avoid that handles are opened and closed automatically
-        each time data is read or written.
-        """
-        if self._is_opened:
-            raise Exception("Cannot open view: the view is already opened")
-        else:
-            self._dataset = Dataset.open(
-                str(self._path)
-            )  # No need to pass the header to the wkw.Dataset
-            self._is_opened = True
-        return self
-
-    def close(self) -> None:
-        """
-        Complementary to `open`, this closes the handles to the data.
-
-        See `open` for more information.
-        """
-        if not self._is_opened:
-            raise Exception("Cannot close View: the view is not opened")
-        else:
-            assert self._dataset is not None  # because the View was opened
-            self._dataset.close()
-            self._dataset = None
-            self._is_opened = False
-
     def write(
         self,
         data: np.ndarray,
@@ -120,7 +88,6 @@ def write(
 
         offset = Vec3Int(offset)
 
-        was_opened = self._is_opened
         # assert the size of the parameter data is not in conflict with the attribute self.size
         data_dims = Vec3Int(data.shape[-3:])
         _assert_positive_dimensions(offset, data_dims)
@@ -135,14 +102,7 @@ def write(
         if self._is_compressed():
             absolute_offset, data = self._handle_compressed_write(absolute_offset, data)
 
-        if not was_opened:
-            self.open()
-        assert self._dataset is not None  # because the View was opened
-
-        self._dataset.write(absolute_offset.to_np(), data)
-
-        if not was_opened:
-            self.close()
+        self._wkw_dataset.write(absolute_offset.to_np(), data)
 
     def read(
         self,
@@ -206,16 +166,7 @@ def _read_without_checks(
         absolute_offset: Vec3Int,
         size: Vec3Int,
     ) -> np.ndarray:
-        was_opened = self._is_opened
-        if not was_opened:
-            self.open()
-        assert self._dataset is not None  # because the View was opened
-
-        data = self._dataset.read(absolute_offset.to_np(), size.to_np())
-
-        if not was_opened:
-            self.close()
-
+        data = self._wkw_dataset.read(absolute_offset.to_np(), size.to_np())
         return data
 
     def get_view(
@@ -593,6 +544,9 @@ def get_dtype(self) -> type:
         return self.header.voxel_type
 
     def __enter__(self) -> "View":
+        warnings.warn(
+            "[DEPRECATION] Entering a View to open it is deprecated. The internal dataset will be opened automatically."
+        )
         return self
 
     def __exit__(
@@ -601,7 +555,7 @@ def __exit__(
         _value: Optional[BaseException],
         _tb: Optional[TracebackType],
     ) -> None:
-        self.close()
+        pass
 
     def __repr__(self) -> str:
         return repr(
@@ -614,6 +568,23 @@ def _mag_view_bounding_box_at_creation(self) -> BoundingBox:
         assert self._mag_view_bbox_at_creation is not None
         return self._mag_view_bbox_at_creation
 
+    @property
+    def _wkw_dataset(self) -> wkw.Dataset:
+        if self._cached_wkw_dataset is None:
+            self._cached_wkw_dataset = Dataset.open(
+                str(self._path)
+            )  # No need to pass the header to the wkw.Dataset
+        return self._cached_wkw_dataset
+
+    @_wkw_dataset.deleter
+    def _wkw_dataset(self) -> None:
+        if self._cached_wkw_dataset is not None:
+            self._cached_wkw_dataset.close()
+            self._cached_wkw_dataset = None
+
+    def __del__(self) -> None:
+        del self._cached_wkw_dataset
+
 
 def _assert_positive_dimensions(offset: Vec3Int, size: Vec3Int) -> None:
     if any(x < 0 for x in offset):