No slashes in layer name (#924)

* Change conversion layer mapping to not allow slashes in layernames anymore.

* Update Changelog.

* Add uncomplete test for slash in layername.

* Add test that checks if a slash is in layername.

Author: markbader

webknossos/Changelog.md

@@ -17,6 +17,7 @@ For upgrade instructions, please check the respective _Breaking Changes_ section
 ### Added
 
 ### Changed
+- The conversion folder structures to layer names does not allow slashes in the layer name anymore. [#918](https://github.com/scalableminds/webknossos-libs/issues/918)
 
 ### Fixed
 - Fixed a bug where compression in add_layer_from_images uses too much memory [#900](https://github.com/scalableminds/webknossos-libs/issues/900)

webknossos/tests/constants.py

@@ -11,8 +11,8 @@
 
 from webknossos.utils import rmtree
 
-TESTDATA_DIR = Path("testdata")
-TESTOUTPUT_DIR = Path("testoutput")
+TESTDATA_DIR = Path(__file__).parent.parent / "testdata"
+TESTOUTPUT_DIR = Path(__file__).parent.parent / "testoutput"
 
 
 MINIO_ROOT_USER = "TtnuieannGt2rGuie2t8Tt7urarg5nauedRndrur"

webknossos/tests/dataset/test_from_images.py

@@ -1,12 +1,15 @@
 import warnings
 from pathlib import Path
+from shutil import copytree
 from typing import Iterator
 
 import numpy as np
 import pytest
 from tifffile import TiffFile
 
 import webknossos as wk
+from tests.constants import TESTDATA_DIR
+from webknossos.dataset import Dataset
 
 
 @pytest.fixture(autouse=True, scope="function")
@@ -18,7 +21,7 @@ def ignore_warnings() -> Iterator:
 
 def test_compare_tifffile(tmp_path: Path) -> None:
     ds = wk.Dataset.from_images(
-        "testdata/tiff",
+        TESTDATA_DIR / "tiff",
         tmp_path,
         (1, 1, 1),
         compress=True,
@@ -30,14 +33,14 @@ def test_compare_tifffile(tmp_path: Path) -> None:
     assert "tiff" in ds.layers
     data = ds.layers["tiff"].get_finest_mag().read()[0, :, :]
     for z_index in range(0, data.shape[-1]):
-        with TiffFile("testdata/tiff/test.0000.tiff") as tif_file:
+        with TiffFile(TESTDATA_DIR / "tiff" / "test.0000.tiff") as tif_file:
             comparison_slice = tif_file.asarray().T
         assert np.array_equal(data[:, :, z_index], comparison_slice)
 
 
 def test_multiple_multitiffs(tmp_path: Path) -> None:
     ds = wk.Dataset.from_images(
-        "testdata/various_tiff_formats",
+        TESTDATA_DIR / "various_tiff_formats",
         tmp_path,
         (1, 1, 1),
     )
@@ -55,3 +58,22 @@ def test_multiple_multitiffs(tmp_path: Path) -> None:
         assert layer.dtype_per_channel == np.dtype(dtype)
         assert layer.num_channels == channels
         assert layer.bounding_box.size == size
+
+
+def test_no_slashes_in_layername(tmp_path: Path) -> None:
+    (input_path := tmp_path / "tiff" / "subfolder" / "tifffiles").mkdir(parents=True)
+    copytree(
+        TESTDATA_DIR / "tiff_with_different_dimensions",
+        input_path,
+        dirs_exist_ok=True,
+    )
+
+    for strategy in Dataset.ConversionLayerMapping:
+        dataset = wk.Dataset.from_images(
+            tmp_path / "tiff",
+            tmp_path / str(strategy),
+            voxel_size=(10, 10, 10),
+            map_filepath_to_layer_name=strategy,
+        )
+
+        assert all("/" not in layername for layername in dataset.layers)

webknossos/tests/test_cli.py

@@ -20,6 +20,7 @@
     MINIO_ROOT_PASSWORD,
     MINIO_ROOT_USER,
     REMOTE_TESTOUTPUT_DIR,
+    TESTDATA_DIR,
     use_minio,
 )
 from webknossos import BoundingBox, DataFormat, Dataset
@@ -43,9 +44,6 @@ def tmp_cwd() -> Iterator[None]:
             os.chdir(prev_cwd)
 
 
-TESTDATA_DIR = Path(__file__).parent.parent / "testdata"
-
-
 @pytest.fixture(autouse=True, scope="module")
 def minio_docker() -> Iterator[None]:
     with use_minio():

webknossos/webknossos/dataset/dataset.py

@@ -180,12 +180,14 @@ def _to_callable(
             ConversionLayerMapping = Dataset.ConversionLayerMapping
 
             if self == ConversionLayerMapping.ENFORCE_LAYER_PER_FILE:
-                return str
+                return lambda p: p.as_posix().replace("/", "_")
             elif self == ConversionLayerMapping.ENFORCE_SINGLE_LAYER:
                 return lambda p: input_path.name
             elif self == ConversionLayerMapping.ENFORCE_LAYER_PER_FOLDER:
                 return (
-                    lambda p: input_path.name if p.parent == Path() else str(p.parent)
+                    lambda p: input_path.name
+                    if p.parent == Path()
+                    else p.parent.as_posix().replace("/", "_")
                 )
             elif self == ConversionLayerMapping.ENFORCE_LAYER_PER_TOPLEVEL_FOLDER:
                 return lambda p: input_path.name if p.parent == Path() else p.parts[0]
@@ -201,7 +203,7 @@ def _to_callable(
                     )
                     else input_path.name
                     if p.parent == Path()
-                    else str(p.parent)
+                    else p.parent.as_posix().replace("/", "_")
                 )
             elif self == ConversionLayerMapping.INSPECT_SINGLE_FILE:
                 # As before, but only a single image is inspected to determine 2D vs 3D.
@@ -213,9 +215,7 @@ def _to_callable(
                     return str
                 else:
                     return (
-                        lambda p: input_path.name
-                        if p.parent == Path()
-                        else str(p.parent)
+                        lambda p: input_path.name if p.parent == Path() else p.parts[-2]
                     )
             else:
                 raise ValueError(f"Got unexpected ConversionLayerMapping value: {self}")

webknossos/webknossos/dataset/layer.py

@@ -231,6 +231,9 @@ def name(self, layer_name: str) -> None:
             layer_name not in self.dataset.layers.keys()
         ), f"Failed to rename layer {self.name} to {layer_name}: The new name already exists."
         assert is_fs_path(self.path), f"Cannot rename remote layer {self.path}"
+        assert (
+            "/" not in layer_name
+        ), f"Cannot rename layer, because there is a '/' character in the layer name: {layer_name}"
         self.path.rename(self.dataset.path / layer_name)
         del self.dataset.layers[self.name]
         self.dataset.layers[layer_name] = self