Deprecate chunks_per_shard in favor of shard_shape (#1257)

* make Zarr3 default DataFormat

* compress=True

* remove deprecations

* test fixes

* down to 16

* changelog

* test fixes

* fix /test_dataset_add_remote_mag_and_layer.py

* stuff

* ci

* ci

* error on fork

* ci

* ci

* less alignment checks

* allow_unaligned

* ci.yml aktualisieren

* ci testing

* ci

* sequential tests

* ci

* ci

* ci

* ci

* parameterize python for kubernetes dockerfile

* test

* change defaults

* mirrored test images

* mp logging

* mp debugging

* debug

* debug

* debug

* debugging

* pyproject.toml

* py3.12

* debugging

* wip

* all python versions

* revert debug changes in cluster_tools

* fixes

* larger ci runner

* default ci runner

* rm pytest-timeout

* test

* Revert "rm pytest-timeout"

This reverts commit 6bc2185.

* Revert "test"

This reverts commit 8d57971.

* ci

* ci

* ci

* ci

* ci

* ci

* properly implement SequentialExecutor

* ci

* changelog

* allow_unaligned wip

* ci

* wip

* fix tests

* fix test

* examples

* longer sleep in slurm test

* format

* longer sleep in slurm test

* Apply suggestions from code review

Co-authored-by: Philipp Otto <[email protected]>

* add methods

* more robust patch

* comment

* derive_nd_bounding_box_from_shape

* refactor Dataset.open to not require an additional IOop

* cassettes

* format

* lint

* docs

* deprecate chunks_per_shard

* deprecate dtype_per_layer

* type

* fixes for add_layer_from_images

* Vec3Int.from_vec_or_int

* export defaults

* write_layer args

* MagLike + mag in write_layer

* doc

* docstring

* change default data_format in cli

* docs

* changelog

* changelog

* Update webknossos/Changelog.md

Co-authored-by: Mark Bader <[email protected]>

* docstring

* more kwargs

* tests

---------

Co-authored-by: Philipp Otto <[email protected]>
Co-authored-by: Mark Bader <[email protected]>

Author: 3 people

webknossos/Changelog.md

@@ -14,6 +14,8 @@ For upgrade instructions, please check the respective _Breaking Changes_ section
 
 ### Breaking Changes
 - Changed writing behavior. There is a new argument `allow_resize` for `MagView.write`, which defaults to `False`. If set to `True`, the bounding box of the underlying `Layer` will be resized to fit the to-be-written data. That largely mirrors the previous behavior. However, it is not safe for concurrent operations, so it is disabled by default. It is recommended to set the `Layer.bounding_box` to the desired size before writing. Additionally, by default, writes need to be aligned with the underlying shard grid to guard against concurrency issues and avoid performance footguns. There is a new argument `allow_unaligned`, which defaults to `False`. If set to `True`, the check for shard alignment is skipped.
+- Deprecated `chunks_per_shard` arguments in favor of `shard_shape`, which equals to `shard_shape = chunk_shape * chunks_per_shard`. The shard shape is more intuitive, because it directly defines the size of shards instead of being a factor of the chunk shape.
+- Deprecated `dtype_per_layer` argument, because it promotes the use of uncommon dtypes and leads to confusion with the other `dtype_per_channel` argument. With this change only the use of `dtype_per_channel` is encouraged.
 - Removed deprecated functions, properties and arguments:
   - Functions:
     - `open_annotation`, use `Annotation.load()` instead
@@ -101,12 +103,58 @@ For upgrade instructions, please check the respective _Breaking Changes_ section
   - `compress` in `Layer.upsample` is now `True`
   - `buffer_size` in `View.get_buffered_slice_reader` is now computed from the shard shape
   - `buffer_size` in `View.get_buffered_slice_writer` is now computed from the shard shape
+- Moved from positional argument to keyword-only argument:
+  - `json_update_allowed` in `MagView.write`
+  - `organization_id`, `sharing_token`, `webknossos_url`, `bbox`, `layers`, `mags`, `path`, `exist_ok` in `Dataset.download`
+  - `layers_to_link`, `jobs` in `Dataset.upload`
+  - `dtype_per_layer`, `dtype_per_channel`, `num_channels`, `data_format`, `bounding_box` in `Dataset.add_layer`
+  - `dtype_per_layer`, `dtype_per_channel`, `num_channels`, `data_format` in `Dataset.get_or_add_layer`
+  - `data_format`, `mag`, `chunk_shape`, `chunks_per_shard`, `shard_shape`, `compress` in `Dataset.add_layer_from_images`
+  - `chunk_shape`, `shard_shape`, `chunks_per_shard`, `data_format`, `compress`, `executor` in `Dataset.add_copy_layer`
+  - `organization_id`, `tags`, `name`, `folder_id` in `Dataset.get_remote_datasets`
+  - `make_relative` in `Dataset.add_symlink_layer`
+  - `name`, `make_relative`, `layers_to_ignore` in `Dataset.shallow_copy_dataset`
+  - `executor` in `Dataset.compress`
+  - `sampling_mode`, `coarsest_mag`, `executor` in `Dataset.downsample`
+  - `voxel_size`, `chunk_shape`, `shard_shape`, `chunks_per_shard`, `data_format`, `compress`, `executor`, `voxel_size_with_unit` in `Dataset.copy_dataset`
+  - `chunk_shape`, `shard_shape`, `chunks_per_shard`, `compress` in `Layer.add_mag`
+  - `chunk_shape`, `shard_shape`, `chunks_per_shard`, `compress` in `Layer.get_or_add_mag`
+  - `extend_layer_bounding_box`, `chunk_shape`, `shard_shape`, `chunks_per_shard`, `compress`, `executor` in `Layer.add_copy_mag`
+  - `make_relative`, `extend_layer_bounding_box` in `Layer.add_symlink_mag`
+  - `extend_layer_bounding_box` in `Layer.add_remote_mag`
+  - `extend_layer_bounding_box` in `Layer.add_fs_copy_mag`
+  - `move`, `extend_layer_bounding_box` in `Layer.add_mag_from_zarrarray`
+  - `from_mag`, `coarsest_mag`, `interpolation_mode`, `compress`, `sampling_mode`, `align_with_other_layers`, `buffer_shape`, `force_sampling_scheme`, `allow_overwrite`, `only_setup_mags`, `executor` in `Layer.downsample`
+  - `interpolation_mode`, `compress`, `buffer_shape`, `allow_overwrite`, `only_setup_mag`, `executor` in `Layer.downsample_mag`
+  - `interpolation_mode`, `compress`, `buffer_shape`, `executor` in `Layer.redownsample`
+  - `interpolation_mode`, `compress`, `buffer_shape`, `allow_overwrite`, `only_setup_mags`, `executor` in `Layer.downsample_mag_list`
+  - `finest_mag`, `compress`, `sampling_mode`, `align_with_other_layers`, `buffer_shape`, `executor` in `Layer.upsample`
+  - `chunk_shape`, `executor` in `SegmentationLayer.refresh_largest_segment_id`
+  - `chunk_shape`, `shard_shape`, `chunks_per_shard`, `compression_mode`, `path` in `MagView.create`
+  - `target_path`, `executor` in `MagView.compress`
 - Added arguments:
   - `allow_resize` in `MagView.write` with default `False`
   - `allow_unaligned` in `MagView.write` with default `False`
+  - `shard_shape` in `Dataset.from_images`
+  - `shard_shape` in `Dataset.add_layer_from_images`
+  - `shard_shape` in `Dataset.copy_dataset`
+- Newly deprecated arguments:
+  - `chunks_per_shard` in `Dataset.from_images`, use `shard_shape` instead
+  - `dtype_per_layer` in `Dataset.add_layer`, use `dtype_per_channel` instead
+  - `dtype_per_layer` in `Dataset.get_or_add_layer`, use `dtype_per_channel` instead
+  - `chunks_per_shard` in `Dataset.add_layer_from_images`, use `shard_shape` instead
+  - `chunks_per_shard` in `Dataset.copy_dataset`, use `shard_shape` instead
+  - `dtype_per_layer` in `Layer.__init__`, use `dtype_per_channel` instead
+  - `chunks_per_shard` in `Layer.add_mag`, use `shard_shape` instead
+  - `chunks_per_shard` in `Layer.get_or_add_mag`, use `shard_shape` instead
+  - `chunks_per_shard` in `Layer.add_copy_mag`, use `shard_shape` instead
+  - `chunks_per_shard` in `MagView.create`, use `shard_shape` instead
+- Newly deprecated properties:
+  - `Layer.dtype_per_layer`
 
 
 ### Added
+- Added `Dataset.write_layer` method for writing entire layers in one go. [#1242](https://github.com/scalableminds/webknossos-libs/pull/1242)
 
 ### Changed
 

webknossos/examples/apply_merger_mode.py

@@ -56,7 +56,7 @@ def main() -> None:
     out_layer = dataset.add_layer(
         "segmentation_remapped",
         wk.SEGMENTATION_CATEGORY,
-        dtype_per_layer=in_layer.dtype_per_layer,
+        dtype_per_channel=in_layer.dtype_per_channel,
         largest_segment_id=in_layer.largest_segment_id,
     )
     out_mag1 = out_layer.add_mag("1")

webknossos/examples/dataset_usage.py

@@ -67,8 +67,8 @@ def main() -> None:
 
     copy_of_dataset = dataset.copy_dataset(
         "testoutput/copy_of_dataset",
-        chunk_shape=8,
-        chunks_per_shard=8,
+        chunk_shape=(8, 8, 8),
+        shard_shape=(64, 64, 64),
         compress=True,
     )
     new_layer = dataset.add_layer(

webknossos/examples/learned_segmenter.py

@@ -66,8 +66,7 @@ def main() -> None:
         segmentation_layer = new_dataset.add_layer(
             "segmentation",
             wk.SEGMENTATION_CATEGORY,
-            segmentation.dtype,
-            compressed=True,
+            dtype_per_channel=segmentation.dtype,
             largest_segment_id=int(segmentation.max()),
         )
         segmentation_layer.bounding_box = dataset.layers["color"].bounding_box

webknossos/tests/dataset/test_add_layer_from_images.py

@@ -32,7 +32,7 @@ def test_compare_tifffile(tmp_path: Path) -> None:
         category="segmentation",
         topleft=(100, 100, 55),
         chunk_shape=(8, 8, 8),
-        chunks_per_shard=(8, 8, 8),
+        shard_shape=(64, 64, 64),
     )
     assert layer.bounding_box.topleft == wk.Vec3Int(100, 100, 55)
     data = layer.get_finest_mag().read()[0, :, :]
@@ -52,7 +52,7 @@ def test_compare_nd_tifffile(tmp_path: Path) -> None:
             topleft=(2, 55, 100, 100),
             data_format="zarr3",
             chunk_shape=(8, 8, 8),
-            chunks_per_shard=(8, 8, 8),
+            shard_shape=(64, 64, 64),
             executor=executor,
         )
     assert layer.bounding_box.topleft == wk.VecInt(
@@ -373,15 +373,15 @@ def test_bioformats(
     (
         "https://static.webknossos.org/data/webknossos-libs/slice_0420.dm4",
         "slice_0420.dm4",
-        {"data_format": "zarr"},  # using zarr to allow z=1 chunking
+        {"data_format": "zarr3"},  # using zarr to allow z=1 chunking
         "uint16",
         1,
         (8192, 8192, 1),
     ),
     (
         "https://static.webknossos.org/data/webknossos-libs/slice_0073.dm3",
         "slice_0073.dm3",
-        {"data_format": "zarr"},  # using zarr to allow z=1 chunking
+        {"data_format": "zarr3"},  # using zarr to allow z=1 chunking
         "uint16",
         1,
         (4096, 4096, 1),
@@ -392,15 +392,15 @@ def test_bioformats(
             "https://static.webknossos.org/data/webknossos-libs/slice_0074.dm3",
         ],
         ["slice_0073.dm3", "slice_0074.dm3"],
-        {"data_format": "zarr"},  # using zarr to allow smaller chunking
+        {"data_format": "zarr3"},  # using zarr to allow smaller chunking
         "uint16",
         1,
         (4096, 4096, 2),
     ),
     (
         "https://static.webknossos.org/data/wklibs-samples/dnasample1.zip",
         "dnasample1.dm3",
-        {"data_format": "zarr"},  # using zarr to allow z=1 chunking
+        {"data_format": "zarr3"},  # using zarr to allow z=1 chunking
         "int16",
         1,
         (4096, 4096, 1),

webknossos/tests/dataset/test_buffered_slice_utils.py

@@ -30,7 +30,7 @@ def test_buffered_slice_writer() -> None:
         dtype_per_channel=dtype,
         bounding_box=BoundingBox(origin, (24, 24, 35)),
     )
-    mag_view = layer.add_mag(mag, chunks_per_shard=(32, 32, 1))
+    mag_view = layer.add_mag(mag, shard_shape=(1024, 1024, 32))
 
     with mag_view.get_buffered_slice_writer(absolute_offset=origin) as writer:
         for i in range(13):
@@ -73,8 +73,8 @@ def test_buffered_slice_writer_along_different_axis(
     cube_size_without_channel = test_cube.shape[1:]
     offset = Vec3Int(64, 96, 32)
 
-    chunks_per_shard = [32, 32, 32]
-    chunks_per_shard[dim] = 1
+    shard_shape = [1024, 1024, 1024]
+    shard_shape[dim] = 32
 
     ds = Dataset(tmp_path / f"buffered_slice_writer_{dim}", voxel_size=(1, 1, 1))
     layer = ds.add_layer(
@@ -83,7 +83,7 @@ def test_buffered_slice_writer_along_different_axis(
         num_channels=test_cube.shape[0],
         bounding_box=BoundingBox(offset, cube_size_without_channel),
     )
-    mag_view = layer.add_mag(1, chunks_per_shard=chunks_per_shard)
+    mag_view = layer.add_mag(1, shard_shape=shard_shape)
 
     with mag_view.get_buffered_slice_writer(
         absolute_offset=offset, buffer_size=5, dimension=dim, allow_unaligned=True
@@ -115,7 +115,7 @@ def test_buffered_slice_reader_along_different_axis(tmp_path: Path) -> None:
             num_channels=3,
             bounding_box=BoundingBox(offset, cube_size_without_channel),
         )
-        mag_view = layer.add_mag(1, chunks_per_shard=(32, 32, 1))
+        mag_view = layer.add_mag(1, shard_shape=(1024, 1024, 32))
         mag_view.write(test_cube, absolute_offset=offset)
 
         with (
@@ -156,7 +156,7 @@ def test_basic_buffered_slice_writer(tmp_path: Path) -> None:
         num_channels=1,
         bounding_box=BoundingBox((0, 0, 0), shape),
     )
-    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 32), chunks_per_shard=(8, 8, 8))
+    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 32), shard_shape=(256, 256, 256))
 
     with warnings.catch_warnings():
         warnings.filterwarnings("error")  # This escalates the warning to an error
@@ -184,7 +184,7 @@ def test_buffered_slice_writer_unaligned(
         num_channels=1,
         bounding_box=BoundingBox((0, 0, 0), (513, 513, 36)),
     )
-    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 32), chunks_per_shard=(8, 8, 8))
+    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 32), shard_shape=(256, 256, 256))
 
     # Write some data to z=32. We will check that this
     # data is left untouched by the buffered slice writer.
@@ -230,7 +230,7 @@ def test_buffered_slice_writer_should_raise_unaligned_usage(
         num_channels=1,
         bounding_box=BoundingBox((0, 0, 0), (513, 513, 33)),
     )
-    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 32), chunks_per_shard=(8, 8, 8))
+    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 32), shard_shape=(256, 256, 256))
 
     offset = (1, 1, 1)
 
@@ -261,7 +261,7 @@ def test_basic_buffered_slice_writer_multi_shard(tmp_path: Path) -> None:
         num_channels=1,
         bounding_box=BoundingBox((0, 0, 0), (160, 150, 140)),
     )
-    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 32), chunks_per_shard=(4, 4, 4))
+    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 32), shard_shape=(128, 128, 128))
     assert mag1.info.shard_shape[2] == 32 * 4
 
     # Allocate some data (~ 3 MB) that covers multiple shards (also in z)
@@ -292,7 +292,7 @@ def test_basic_buffered_slice_writer_multi_shard_multi_channel(tmp_path: Path) -
         num_channels=3,
         bounding_box=BoundingBox((0, 0, 0), (160, 150, 140)),
     )
-    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 32), chunks_per_shard=(4, 4, 4))
+    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 32), shard_shape=(128, 128, 128))
 
     # Allocate some data (~ 3 MB) that covers multiple shards (also in z)
     shape = (3, 160, 150, 140)
@@ -319,7 +319,7 @@ def test_buffered_slice_writer_reset_offset(tmp_path: Path) -> None:
         num_channels=1,
         bounding_box=BoundingBox((0, 0, 0), (512, 512, 40)),
     )
-    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 8), chunks_per_shard=(8, 8, 1))
+    mag1 = layer.add_mag("1", chunk_shape=(32, 32, 8), shard_shape=(256, 256, 8))
 
     # Allocate some data (~ 8 MB)
     shape = (512, 512, 32)