From 878b946d88c24f1059ffa22c9f542b901c96bc1a Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Wed, 18 Oct 2023 16:10:37 +0200
Subject: [PATCH 01/30] BROKEN use BIA-data test to catch error in #578

---
 tests/tasks/test_import_ome_zarr.py | 39 ++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/tests/tasks/test_import_ome_zarr.py b/tests/tasks/test_import_ome_zarr.py
index a2589e2dc..5b420398e 100644
--- a/tests/tasks/test_import_ome_zarr.py
+++ b/tests/tasks/test_import_ome_zarr.py
@@ -1,8 +1,15 @@
-import pytest
+# import pytest
 import zarr
 from devtools import debug
 
+import fractal_tasks_core.tasks  # noqa
 from .._zenodo_ome_zarrs import prepare_3D_zarr
+from .test_workflows_cellpose_segmentation import (
+    cellpose_segmentation,
+)  # FIXME import
+from .test_workflows_cellpose_segmentation import patched_cellpose_core_use_gpu
+from .test_workflows_cellpose_segmentation import patched_segment_ROI
+from fractal_tasks_core.lib_input_models import Channel
 from fractal_tasks_core.tasks.copy_ome_zarr import copy_ome_zarr
 from fractal_tasks_core.tasks.import_ome_zarr import import_ome_zarr
 from fractal_tasks_core.tasks.maximum_intensity_projection import (
@@ -130,8 +137,8 @@ def test_import_ome_zarr_image(tmp_path, zenodo_zarr, zenodo_zarr_metadata):
     _check_ROI_tables(f"{root_path}/{zarr_name}")
 
 
-@pytest.mark.skip
-def test_import_ome_zarr_image_BIA(tmp_path):
+# @pytest.mark.skip
+def test_import_ome_zarr_image_BIA(tmp_path, monkeypatch):
     """
     This test imports one of the BIA OME-Zarr listed in
     https://www.ebi.ac.uk/biostudies/bioimages/studies/S-BIAD843.
@@ -193,3 +200,29 @@ def test_import_ome_zarr_image_BIA(tmp_path):
         image_ROI_table[:, "len_x_micrometer"].X[0, 0],
         EXPECTED_X_LENGTH,
     )
+
+    monkeypatch.setattr(
+        "fractal_tasks_core.tasks.cellpose_segmentation.cellpose.core.use_gpu",
+        patched_cellpose_core_use_gpu,
+    )
+
+    monkeypatch.setattr(
+        "fractal_tasks_core.tasks.cellpose_segmentation.segment_ROI",
+        patched_segment_ROI,
+    )
+
+    # Per-FOV labeling
+    for component in metadata["image"]:
+        cellpose_segmentation(
+            input_paths=[str(root_path)],
+            output_path=str(root_path),
+            metadata=metadata,
+            component=component,
+            channel=Channel(label="Channel 0"),
+            level=1,
+            relabeling=True,
+            diameter_level0=80.0,
+            augment=True,
+            net_avg=True,
+            min_size=30,
+        )

From b9c2671cdf500016f05866f1555a9c5ef35e2784 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 10:09:38 +0200
Subject: [PATCH 02/30] Add `update_omero_channels` helper function and its
 unit test (ref #578)

---
 fractal_tasks_core/lib_channels.py | 80 ++++++++++++++++++++++++++++++
 tests/test_unit_channels.py        | 27 ++++++++++
 2 files changed, 107 insertions(+)

diff --git a/fractal_tasks_core/lib_channels.py b/fractal_tasks_core/lib_channels.py
index 811830f8c..e881e784d 100644
--- a/fractal_tasks_core/lib_channels.py
+++ b/fractal_tasks_core/lib_channels.py
@@ -12,6 +12,8 @@
 Helper functions to address channels via OME-NGFF/OMERO metadata.
 """
 import logging
+from copy import deepcopy
+from typing import Any
 from typing import Optional
 from typing import Union
 
@@ -339,3 +341,81 @@ def define_omero_channels(
     ]
 
     return new_channels_dictionaries
+
+
+def _get_new_unique_value(
+    value: str,
+    existing_values: list[str],
+) -> str:
+    """
+    Produce a string value that is not present in a given list
+
+    Append `_1`, `_2`, ... to a given string, if needed, until finding a value
+    which is not already present in `existing_values`.
+
+    Args:
+        value: The first guess for the new value
+        existing_values: The list of existing values
+
+    Returns:
+        A string value which is not present in `existing_values`
+    """
+    counter = 1
+    new_value = value
+    while new_value in existing_values:
+        new_value = f"{value}-{counter}"
+        counter += 1
+    return new_value
+
+
+def update_omero_channels(
+    old_channels: list[dict[str, Any]]
+) -> list[dict[str, Any]]:
+    """
+    Make an existing list of Omero channels Fractal-compatible
+
+    The output channels all have `label` and `wavelength_id` keys, with the
+    `wavelength_id` values being unique across the channel list.
+
+    See https://ngff.openmicroscopy.org/0.4/index.html#omero-md for the
+    definition of NGFF Omero metadata.
+
+    Args:
+        old_channels: Existing list of Omero-channel dictionaries
+
+    Returns:
+        New list of Fractal-compatible Omero-channel dictionaries
+    """
+    new_channels = deepcopy(old_channels)
+    existing_wavelength_ids = []
+
+    # Step 1: handle channels that already have a "label" attribute
+    for ind, old_channel in enumerate(old_channels):
+        if "label" not in old_channel.keys():
+            continue
+        label = old_channel["label"]
+        wavelength_id = _get_new_unique_value(
+            label,
+            existing_wavelength_ids,
+        )
+        existing_wavelength_ids.append(wavelength_id)
+        new_channel = old_channel.copy()
+        new_channel["wavelength_id"] = wavelength_id
+        new_channels[ind] = new_channel
+
+    # Step 2: handle channels that do not have a "label" attribute
+    for ind, old_channel in enumerate(old_channels):
+        if "label" in old_channel.keys():
+            continue
+        label = str(ind + 1)
+        wavelength_id = _get_new_unique_value(
+            label,
+            existing_wavelength_ids,
+        )
+        existing_wavelength_ids.append(wavelength_id)
+        new_channel = old_channel.copy()
+        new_channel["label"] = label
+        new_channel["wavelength_id"] = wavelength_id
+        new_channels[ind] = new_channel
+
+    return new_channels
diff --git a/tests/test_unit_channels.py b/tests/test_unit_channels.py
index 9ae34bdf0..20a34765e 100644
--- a/tests/test_unit_channels.py
+++ b/tests/test_unit_channels.py
@@ -11,6 +11,7 @@
 from fractal_tasks_core.lib_channels import define_omero_channels
 from fractal_tasks_core.lib_channels import get_channel_from_list
 from fractal_tasks_core.lib_channels import OmeroChannel
+from fractal_tasks_core.lib_channels import update_omero_channels
 
 
 def test_check_unique_wavelength_ids():
@@ -196,3 +197,29 @@ def test_color_validator():
     for c in invalid_colors:
         with pytest.raises(ValueError):
             OmeroChannel(wavelength_id="A01_C01", color=c)
+
+
+@pytest.mark.parametrize(
+    "old_channels",
+    [
+        [{}, {}, {}],
+        [{"label": "A"}, {"label": "B"}, {"label": "C"}],
+        [{"label": "A"}, {}, {"label": "C"}],
+        [{"label": "A"}, {"label": "A"}, {"label": "A"}],
+        [{"label": "1"}, {"label": "1"}, {"label": "1"}],
+        [{}, {"label": "1"}, {}, {"label": "1"}],
+    ],
+)
+def test_update_omero_channels(old_channels):
+
+    # Update partial metadata
+    print()
+    print(f"OLD: {old_channels}")
+    new_channels = update_omero_channels(old_channels)
+    print(f"NEW: {new_channels}")
+
+    # Validate new channels as `OmeroChannel` objects, and check that they
+    # have unique `wavelength_id` values
+    check_unique_wavelength_ids(
+        [OmeroChannel(**channel) for channel in new_channels]
+    )

From c62a812321d29c343210b18658cb79fabe8fec57 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 10:24:37 +0200
Subject: [PATCH 03/30] Implement first version of `update_omero_metadata` in
 `import_ome_zarr` task (ref #578)

---
 fractal_tasks_core/__FRACTAL_MANIFEST__.json |  6 ++++
 fractal_tasks_core/tasks/import_ome_zarr.py  | 29 ++++++++++++++++++--
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/fractal_tasks_core/__FRACTAL_MANIFEST__.json b/fractal_tasks_core/__FRACTAL_MANIFEST__.json
index 7877cf068..6e14d1a40 100644
--- a/fractal_tasks_core/__FRACTAL_MANIFEST__.json
+++ b/fractal_tasks_core/__FRACTAL_MANIFEST__.json
@@ -1245,6 +1245,12 @@
             "type": "integer",
             "description": "X shape of the ROI grid in `grid_ROI_table`."
           },
+          "update_omero_metadata": {
+            "title": "Update Omero Metadata",
+            "default": true,
+            "type": "boolean",
+            "description": "Whether to update Omero-channels metadata, to make them Fractal-compatible."
+          },
           "overwrite": {
             "title": "Overwrite",
             "default": false,
diff --git a/fractal_tasks_core/tasks/import_ome_zarr.py b/fractal_tasks_core/tasks/import_ome_zarr.py
index 73ee05121..82cc4744a 100644
--- a/fractal_tasks_core/tasks/import_ome_zarr.py
+++ b/fractal_tasks_core/tasks/import_ome_zarr.py
@@ -21,6 +21,7 @@
 import zarr
 from pydantic.decorator import validate_arguments
 
+from fractal_tasks_core.lib_channels import update_omero_channels
 from fractal_tasks_core.lib_ngff import detect_ome_ngff_type
 from fractal_tasks_core.lib_ngff import NgffImageMeta
 from fractal_tasks_core.lib_regions_of_interest import get_image_grid_ROIs
@@ -34,6 +35,8 @@ def _process_single_image(
     image_path: str,
     add_image_ROI_table: bool,
     add_grid_ROI_table: bool,
+    update_omero_metadata: bool,
+    *,
     grid_YX_shape: Optional[tuple[int, int]] = None,
     overwrite: bool = False,
 ) -> None:
@@ -51,6 +54,8 @@ def _process_single_image(
             (argument propagated from `import_ome_zarr`).
         add_grid_ROI_table: Whether to add a `grid_ROI_table` table (argument
             propagated from `import_ome_zarr`).
+        update_omero_metadata: Whether to update Omero-channels metadata
+            (argument propagated from `import_ome_zarr`).
         grid_YX_shape: YX shape of the ROI grid (it must be not `None`, if
             `add_grid_ROI_table=True`.
     """
@@ -100,6 +105,18 @@ def _process_single_image(
             logger=logger,
         )
 
+    # Update Omero-channels metadata
+    if update_omero_metadata:
+        if image_meta.omero is None or image_meta.omero.channels == []:
+            # TODO: create omero-channels list from scratch
+            raise NotImplementedError
+        old_channels = [c.dict() for c in image_meta.omero.channels]
+        new_channels = update_omero_channels(old_channels)
+        old_omero = image_group.attrs["omero"]
+        new_omero = old_omero.copy()
+        new_omero["channels"] = new_channels
+        image_group.attrs.update(omero=new_omero)
+
 
 @validate_arguments
 def import_ome_zarr(
@@ -112,6 +129,7 @@ def import_ome_zarr(
     add_grid_ROI_table: bool = True,
     grid_y_shape: int = 2,
     grid_x_shape: int = 2,
+    update_omero_metadata: bool = True,
     overwrite: bool = False,
 ) -> dict[str, Any]:
     """
@@ -141,6 +159,8 @@ def import_ome_zarr(
             image, with the image split into a rectangular grid of ROIs.
         grid_y_shape: Y shape of the ROI grid in `grid_ROI_table`.
         grid_x_shape: X shape of the ROI grid in `grid_ROI_table`.
+        update_omero_metadata: Whether to update Omero-channels metadata, to
+            make them Fractal-compatible.
         overwrite: Whether new ROI tables (added when `add_image_ROI_table`
             and/or `add_grid_ROI_table` are `True`) can overwite existing ones.
     """
@@ -174,7 +194,8 @@ def import_ome_zarr(
                     f"{zarr_path}/{well_path}/{image_path}",
                     add_image_ROI_table,
                     add_grid_ROI_table,
-                    grid_YX_shape,
+                    update_omero_metadata,
+                    grid_YX_shape=grid_YX_shape,
                     overwrite=overwrite,
                 )
     elif ngff_type == "well":
@@ -191,7 +212,8 @@ def import_ome_zarr(
                 f"{zarr_path}/{image_path}",
                 add_image_ROI_table,
                 add_grid_ROI_table,
-                grid_YX_shape,
+                update_omero_metadata,
+                grid_YX_shape=grid_YX_shape,
                 overwrite=overwrite,
             )
     elif ngff_type == "image":
@@ -205,7 +227,8 @@ def import_ome_zarr(
             zarr_path,
             add_image_ROI_table,
             add_grid_ROI_table,
-            grid_YX_shape,
+            update_omero_metadata,
+            grid_YX_shape=grid_YX_shape,
             overwrite=overwrite,
         )
 

From cb11bec5439dcb961af2e4e1aa7c22420de1a013 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 10:25:41 +0200
Subject: [PATCH 04/30] Update `test_import_ome_zarr_image_BIA` (ref #578)

Note: the test still fails, but it is because of #169 (that is, lacking support for time axis)
---
 tests/tasks/test_import_ome_zarr.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/tasks/test_import_ome_zarr.py b/tests/tasks/test_import_ome_zarr.py
index 5b420398e..d6d6d415c 100644
--- a/tests/tasks/test_import_ome_zarr.py
+++ b/tests/tasks/test_import_ome_zarr.py
@@ -216,10 +216,11 @@ def test_import_ome_zarr_image_BIA(tmp_path, monkeypatch):
         cellpose_segmentation(
             input_paths=[str(root_path)],
             output_path=str(root_path),
+            input_ROI_table="grid_ROI_table",
             metadata=metadata,
             component=component,
-            channel=Channel(label="Channel 0"),
-            level=1,
+            channel=Channel(wavelength_id="Channel 0"),
+            level=0,
             relabeling=True,
             diameter_level0=80.0,
             augment=True,

From bd323f0142f60c10f2202f6c4542c88dad878c4e Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 10:30:23 +0200
Subject: [PATCH 05/30] Skip `test_import_ome_zarr_image_BIA`

---
 tests/tasks/test_import_ome_zarr.py | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/tests/tasks/test_import_ome_zarr.py b/tests/tasks/test_import_ome_zarr.py
index d6d6d415c..4fcabc1fa 100644
--- a/tests/tasks/test_import_ome_zarr.py
+++ b/tests/tasks/test_import_ome_zarr.py
@@ -1,14 +1,9 @@
-# import pytest
+import pytest
 import zarr
 from devtools import debug
 
 import fractal_tasks_core.tasks  # noqa
 from .._zenodo_ome_zarrs import prepare_3D_zarr
-from .test_workflows_cellpose_segmentation import (
-    cellpose_segmentation,
-)  # FIXME import
-from .test_workflows_cellpose_segmentation import patched_cellpose_core_use_gpu
-from .test_workflows_cellpose_segmentation import patched_segment_ROI
 from fractal_tasks_core.lib_input_models import Channel
 from fractal_tasks_core.tasks.copy_ome_zarr import copy_ome_zarr
 from fractal_tasks_core.tasks.import_ome_zarr import import_ome_zarr
@@ -137,7 +132,7 @@ def test_import_ome_zarr_image(tmp_path, zenodo_zarr, zenodo_zarr_metadata):
     _check_ROI_tables(f"{root_path}/{zarr_name}")
 
 
-# @pytest.mark.skip
+@pytest.mark.skip
 def test_import_ome_zarr_image_BIA(tmp_path, monkeypatch):
     """
     This test imports one of the BIA OME-Zarr listed in
@@ -145,6 +140,9 @@ def test_import_ome_zarr_image_BIA(tmp_path, monkeypatch):
 
     It is currently marked as "skip", to avoid incurring into download-rate
     limits.
+
+    Also note that any further processing of the imported Zarr this will fail
+    because we don't support time data, see fractal-tasks-core issue #169.
     """
 
     from ftplib import FTP
@@ -201,6 +199,14 @@ def test_import_ome_zarr_image_BIA(tmp_path, monkeypatch):
         EXPECTED_X_LENGTH,
     )
 
+    # Part 2: run Cellpose on the imported OME-Zarr.
+
+    from fractal_tasks_core.cellpose_segmentation import cellpose_segmentation
+    from .test_workflows_cellpose_segmentation import (
+        patched_cellpose_core_use_gpu,
+        patched_segment_ROI,
+    )
+
     monkeypatch.setattr(
         "fractal_tasks_core.tasks.cellpose_segmentation.cellpose.core.use_gpu",
         patched_cellpose_core_use_gpu,

From be5b82201d2619d7bd82ea42b0897eec939ab746 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 10:32:18 +0200
Subject: [PATCH 06/30] Update CHANGELOG

---
 CHANGELOG.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5905abba0..1ab25d4a5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,9 +4,10 @@
 
 * Tasks:
     * New task and helper functions:
-        * Introduce `import_ome_zarr` task (\#557).
+        * Introduce `import_ome_zarr` task (\#557, \#579).
         * Introduce `get_single_image_ROI` and `get_image_grid_ROIs` (\#557).
         * Introduce `detect_ome_ngff_type` (\#557).
+        * Introduce `update_omero_channels` (\#579).
     * Make `maximum_intensity_projection` task not depend on ROI tables (\#557).
     * Make Cellpose task work when `input_ROI_table` is empty (\#566).
     * Fix bug of missing attributes in ROI-table Zarr group (\#573).

From 96197ac2eff243f48ae4d1736085df9758480990 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 10:49:35 +0200
Subject: [PATCH 07/30] Improve handling of omero metadata in import-ome-zarr
 task (ref #578)

---
 fractal_tasks_core/tasks/import_ome_zarr.py | 35 +++++++++++++++++----
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/fractal_tasks_core/tasks/import_ome_zarr.py b/fractal_tasks_core/tasks/import_ome_zarr.py
index 82cc4744a..9795d58cf 100644
--- a/fractal_tasks_core/tasks/import_ome_zarr.py
+++ b/fractal_tasks_core/tasks/import_ome_zarr.py
@@ -46,7 +46,8 @@ def _process_single_image(
     This task:
 
     1. Validates OME-NGFF image metadata, via `NgffImageMeta`;
-    2. Optionally generates and writes two ROI tables.
+    2. Optionally generates and writes two ROI tables;
+    3. Optionally update OME-NGFF omero metadata.
 
     Args:
         image_path: Absolute path to the image Zarr group.
@@ -107,12 +108,34 @@ def _process_single_image(
 
     # Update Omero-channels metadata
     if update_omero_metadata:
-        if image_meta.omero is None or image_meta.omero.channels == []:
-            # TODO: create omero-channels list from scratch
-            raise NotImplementedError
-        old_channels = [c.dict() for c in image_meta.omero.channels]
+
+        # Extract number of channels from zarr array
+        try:
+            channel_axis_index = image_meta.axes_names.index("c")
+        except ValueError:
+            raise NotImplementedError(
+                f"Existing axes ({image_meta.axes_names}) do not "
+                'include "c".'
+            )
+        num_channels_zarr = array.shape[channel_axis_index]
+        print(num_channels_zarr)
+
+        old_omero = image_group.attrs.get("omero", {})
+        old_channels = old_omero.get("channels", [])
+        if old_channels != []:
+            # Case 1: omero/channels exist
+            if len(old_channels) != num_channels_zarr:
+                error_msg = (
+                    "Channels-number mismatch: Number of channels in the "
+                    f"zarr array ({num_channels_zarr}) differ from number "
+                    "of channels listed in NGFF omero metadata "
+                    f"({len(old_channels)})."
+                )
+                raise ValueError(error_msg)
+        else:
+            # Case 2: omero or omero/channels do not exist
+            old_channels = [{} for ind in range(num_channels_zarr)]
         new_channels = update_omero_channels(old_channels)
-        old_omero = image_group.attrs["omero"]
         new_omero = old_omero.copy()
         new_omero["channels"] = new_channels
         image_group.attrs.update(omero=new_omero)

From bf5cf5899fb92341f047e0eae1e454cbabf1a4fe Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 11:25:37 +0200
Subject: [PATCH 08/30] Update test_import_ome_zarr_image_BIA

---
 tests/tasks/test_import_ome_zarr.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/tasks/test_import_ome_zarr.py b/tests/tasks/test_import_ome_zarr.py
index 4fcabc1fa..3ec896588 100644
--- a/tests/tasks/test_import_ome_zarr.py
+++ b/tests/tasks/test_import_ome_zarr.py
@@ -199,6 +199,14 @@ def test_import_ome_zarr_image_BIA(tmp_path, monkeypatch):
         EXPECTED_X_LENGTH,
     )
 
+    g = zarr.open(f"{root_path}/{zarr_name}", mode="r")
+    omero_channels = g.attrs["omero"]["channels"]
+    debug(omero_channels)
+    assert len(omero_channels) == 1
+    omero_channel = omero_channels[0]
+    assert omero_channel["label"] == "Channel 0"
+    assert omero_channel["wavelength_id"] == "Channel 0"
+
     # Part 2: run Cellpose on the imported OME-Zarr.
 
     from fractal_tasks_core.cellpose_segmentation import cellpose_segmentation

From cf2d808938b3ed45f440897d6857fd7c900f4498 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 11:42:53 +0200
Subject: [PATCH 09/30] Add logging in update_omero_channels

---
 fractal_tasks_core/lib_channels.py | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/fractal_tasks_core/lib_channels.py b/fractal_tasks_core/lib_channels.py
index e881e784d..66e165ef9 100644
--- a/fractal_tasks_core/lib_channels.py
+++ b/fractal_tasks_core/lib_channels.py
@@ -387,7 +387,7 @@ def update_omero_channels(
         New list of Fractal-compatible Omero-channel dictionaries
     """
     new_channels = deepcopy(old_channels)
-    existing_wavelength_ids = []
+    existing_wavelength_ids: list[str] = []
 
     # Step 1: handle channels that already have a "label" attribute
     for ind, old_channel in enumerate(old_channels):
@@ -418,4 +418,21 @@ def update_omero_channels(
         new_channel["wavelength_id"] = wavelength_id
         new_channels[ind] = new_channel
 
+    # Step 3: log all label/wavelength_id additions
+    for ind, old_channel in enumerate(old_channels):
+        try:
+            label = old_channel["label"]
+            wavelength_id = new_channels[ind]["wavelength_id"]
+            logging.info(
+                f"Omero channel has {label=}; "
+                f"new attributes: {wavelength_id=}."
+            )
+        except KeyError:
+            label = new_channels[ind]["label"]
+            wavelength_id = new_channels[ind]["wavelength_id"]
+            logging.info(
+                "Omero channel has no label; "
+                f"new attributes: {label=}, {wavelength_id=}."
+            )
+
     return new_channels

From 3d440f0a60e9d334a60e6e18a686a30fc0143bdc Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 11:43:12 +0200
Subject: [PATCH 10/30] Improve logging in `_process_single_image`

---
 fractal_tasks_core/tasks/import_ome_zarr.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/fractal_tasks_core/tasks/import_ome_zarr.py b/fractal_tasks_core/tasks/import_ome_zarr.py
index 9795d58cf..f150831ba 100644
--- a/fractal_tasks_core/tasks/import_ome_zarr.py
+++ b/fractal_tasks_core/tasks/import_ome_zarr.py
@@ -118,11 +118,16 @@ def _process_single_image(
                 'include "c".'
             )
         num_channels_zarr = array.shape[channel_axis_index]
-        print(num_channels_zarr)
-
+        logger.info(
+            f"{num_channels_zarr} channel(s) found in Zarr array "
+            f"at {image_path}/{dataset_subpath}"
+        )
         old_omero = image_group.attrs.get("omero", {})
         old_channels = old_omero.get("channels", [])
-        if old_channels != []:
+        if len(old_channels) > 0:
+            logger.info(
+                f"{len(old_channels)} channel(s) found in NGFF omero metadata"
+            )
             # Case 1: omero/channels exist
             if len(old_channels) != num_channels_zarr:
                 error_msg = (
@@ -131,6 +136,7 @@ def _process_single_image(
                     "of channels listed in NGFF omero metadata "
                     f"({len(old_channels)})."
                 )
+                logging.error(error_msg)
                 raise ValueError(error_msg)
         else:
             # Case 2: omero or omero/channels do not exist
@@ -225,7 +231,7 @@ def import_ome_zarr(
         zarrurls["well"].append(zarr_name)
         logger.warning(
             "Only OME-Zarr for plates are fully supported in Fractal; "
-            "e.g. the current one ({ngff_type=}) cannot be "
+            f"e.g. the current one ({ngff_type=}) cannot be "
             "processed via the `maximum_intensity_projection` task."
         )
         for image in root_group.attrs["well"]["images"]:
@@ -243,7 +249,7 @@ def import_ome_zarr(
         zarrurls["image"].append(zarr_name)
         logger.warning(
             "Only OME-Zarr for plates are fully supported in Fractal; "
-            "e.g. the current one ({ngff_type=}) cannot be "
+            f"e.g. the current one ({ngff_type=}) cannot be "
             "processed via the `maximum_intensity_projection` task."
         )
         _process_single_image(

From 2f19419ba929832e6a95d70fe9af9966ada2db2b Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 11:43:39 +0200
Subject: [PATCH 11/30] Add `remove_omero` option to `prepare_3D_zarr` test
 function

---
 tests/_zenodo_ome_zarrs.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/_zenodo_ome_zarrs.py b/tests/_zenodo_ome_zarrs.py
index 607f5f1c9..f73bab122 100644
--- a/tests/_zenodo_ome_zarrs.py
+++ b/tests/_zenodo_ome_zarrs.py
@@ -12,11 +12,13 @@
 Zurich.
 """
 import json
+import logging
 import shutil
 from pathlib import Path
 from typing import Any
 
 import dask.array as da
+import zarr
 from devtools import debug
 
 
@@ -25,6 +27,7 @@ def prepare_3D_zarr(
     zenodo_zarr: list[str],
     zenodo_zarr_metadata: list[dict[str, Any]],
     remove_tables: bool = False,
+    remove_omero: bool = False,
 ):
     zenodo_zarr_3D, zenodo_zarr_2D = zenodo_zarr[:]
     metadata_3D, metadata_2D = zenodo_zarr_metadata[:]
@@ -35,6 +38,16 @@ def prepare_3D_zarr(
         shutil.rmtree(
             str(Path(zarr_path) / Path(zenodo_zarr_3D).name / "B/03/0/tables")
         )
+        logging.warning("Removing ROI tables attributes from zenodo zarr")
+    if remove_omero:
+        image_group = zarr.open_group(
+            str(Path(zarr_path) / Path(zenodo_zarr_3D).name / "B/03/0"),
+            mode="r+",
+        )
+        image_attrs = image_group.attrs.asdict()
+        image_attrs.pop("omero")
+        image_group.attrs.put(image_attrs)
+        logging.warning("Removing omero attributes from zenodo zarr")
     metadata = metadata_3D.copy()
     return metadata
 

From 975ea42035793d5849ada1db8b19c6ff4de062f3 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 11:43:54 +0200
Subject: [PATCH 12/30] Also test omero-channels addition in
 `test_import_ome_zarr_image`

---
 tests/tasks/test_import_ome_zarr.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/tasks/test_import_ome_zarr.py b/tests/tasks/test_import_ome_zarr.py
index 3ec896588..4a1b3b838 100644
--- a/tests/tasks/test_import_ome_zarr.py
+++ b/tests/tasks/test_import_ome_zarr.py
@@ -107,7 +107,11 @@ def test_import_ome_zarr_image(tmp_path, zenodo_zarr, zenodo_zarr_metadata):
     # Prepare an on-disk OME-Zarr at the plate level
     root_path = tmp_path
     prepare_3D_zarr(
-        root_path, zenodo_zarr, zenodo_zarr_metadata, remove_tables=True
+        root_path,
+        zenodo_zarr,
+        zenodo_zarr_metadata,
+        remove_tables=True,
+        remove_omero=True,
     )
     zarr_name = "plate.zarr/B/03/0"
 
@@ -131,6 +135,12 @@ def test_import_ome_zarr_image(tmp_path, zenodo_zarr, zenodo_zarr_metadata):
     # Check that table were created
     _check_ROI_tables(f"{root_path}/{zarr_name}")
 
+    # Check that omero attributes were filled correctly
+    g = zarr.open_group(str(root_path / zarr_name), mode="r")
+    debug(g.attrs["omero"]["channels"])
+    EXPECTED_CHANNELS = [dict(label="1", wavelength_id="1")]
+    assert g.attrs["omero"]["channels"] == EXPECTED_CHANNELS
+
 
 @pytest.mark.skip
 def test_import_ome_zarr_image_BIA(tmp_path, monkeypatch):

From 4ebcb1b1c53e234b5f8ada560882975558b78e38 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 11:52:10 +0200
Subject: [PATCH 13/30] Add failing test for omero-channels update

---
 tests/tasks/test_import_ome_zarr.py | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/tests/tasks/test_import_ome_zarr.py b/tests/tasks/test_import_ome_zarr.py
index 4a1b3b838..fc51e61d0 100644
--- a/tests/tasks/test_import_ome_zarr.py
+++ b/tests/tasks/test_import_ome_zarr.py
@@ -102,7 +102,10 @@ def test_import_ome_zarr_well(tmp_path, zenodo_zarr, zenodo_zarr_metadata):
     _check_ROI_tables(f"{root_path}/{zarr_name}/0")
 
 
-def test_import_ome_zarr_image(tmp_path, zenodo_zarr, zenodo_zarr_metadata):
+@pytest.mark.parametrize("reset_omero", [True, False])
+def test_import_ome_zarr_image(
+    tmp_path, zenodo_zarr, zenodo_zarr_metadata, reset_omero
+):
 
     # Prepare an on-disk OME-Zarr at the plate level
     root_path = tmp_path
@@ -111,7 +114,7 @@ def test_import_ome_zarr_image(tmp_path, zenodo_zarr, zenodo_zarr_metadata):
         zenodo_zarr,
         zenodo_zarr_metadata,
         remove_tables=True,
-        remove_omero=True,
+        remove_omero=reset_omero,
     )
     zarr_name = "plate.zarr/B/03/0"
 
@@ -138,8 +141,17 @@ def test_import_ome_zarr_image(tmp_path, zenodo_zarr, zenodo_zarr_metadata):
     # Check that omero attributes were filled correctly
     g = zarr.open_group(str(root_path / zarr_name), mode="r")
     debug(g.attrs["omero"]["channels"])
-    EXPECTED_CHANNELS = [dict(label="1", wavelength_id="1")]
-    assert g.attrs["omero"]["channels"] == EXPECTED_CHANNELS
+    if reset_omero:
+        EXPECTED_CHANNELS = [dict(label="1", wavelength_id="1")]
+        assert g.attrs["omero"]["channels"] == EXPECTED_CHANNELS
+    else:
+        EXPECTED_LABEL = "DAPI"
+        EXPECTED_WAVELENGTH_ID = "A01_C01"
+        assert g.attrs["omero"]["channels"][0]["label"] == EXPECTED_LABEL
+        assert (
+            g.attrs["omero"]["channels"][0]["wavelength_id"]
+            == EXPECTED_WAVELENGTH_ID
+        )  # noqa
 
 
 @pytest.mark.skip

From 7f01182b5cdb9b10737c0683816fdcc7ab6f9548 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 11:54:02 +0200
Subject: [PATCH 14/30] Handle existing `wavelength_id`s in omero channels
 correctly

---
 fractal_tasks_core/lib_channels.py | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/fractal_tasks_core/lib_channels.py b/fractal_tasks_core/lib_channels.py
index 66e165ef9..424a94bf6 100644
--- a/fractal_tasks_core/lib_channels.py
+++ b/fractal_tasks_core/lib_channels.py
@@ -394,11 +394,14 @@ def update_omero_channels(
         if "label" not in old_channel.keys():
             continue
         label = old_channel["label"]
-        wavelength_id = _get_new_unique_value(
-            label,
-            existing_wavelength_ids,
-        )
-        existing_wavelength_ids.append(wavelength_id)
+        try:
+            wavelength_id = old_channel["wavelength_id"]
+        except KeyError:
+            wavelength_id = _get_new_unique_value(
+                label,
+                existing_wavelength_ids,
+            )
+            existing_wavelength_ids.append(wavelength_id)
         new_channel = old_channel.copy()
         new_channel["wavelength_id"] = wavelength_id
         new_channels[ind] = new_channel
@@ -408,10 +411,13 @@ def update_omero_channels(
         if "label" in old_channel.keys():
             continue
         label = str(ind + 1)
-        wavelength_id = _get_new_unique_value(
-            label,
-            existing_wavelength_ids,
-        )
+        try:
+            wavelength_id = old_channel["wavelength_id"]
+        except KeyError:
+            wavelength_id = _get_new_unique_value(
+                label,
+                existing_wavelength_ids,
+            )
         existing_wavelength_ids.append(wavelength_id)
         new_channel = old_channel.copy()
         new_channel["label"] = label

From 64823dfb8facbd001d51eaaf156ae1a0a7f1fd4e Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 12:18:14 +0200
Subject: [PATCH 15/30] Expand test_update_omero_channels

---
 tests/test_unit_channels.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/test_unit_channels.py b/tests/test_unit_channels.py
index 20a34765e..a9abca5bd 100644
--- a/tests/test_unit_channels.py
+++ b/tests/test_unit_channels.py
@@ -208,6 +208,12 @@ def test_color_validator():
         [{"label": "A"}, {"label": "A"}, {"label": "A"}],
         [{"label": "1"}, {"label": "1"}, {"label": "1"}],
         [{}, {"label": "1"}, {}, {"label": "1"}],
+        [
+            {"wavelength_id": "1"},
+            {"label": "1"},
+            {},
+            {"label": "1", "wavelength_id": "3"},
+        ],
     ],
 )
 def test_update_omero_channels(old_channels):

From 9c2b0eeda2be6da6444521ed50208030b817275c Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 12:26:47 +0200
Subject: [PATCH 16/30] Refactor `update_omero_channels`, to better handle
 existing `wavelength_id/label` values

---
 fractal_tasks_core/lib_channels.py | 76 +++++++++++++++++-------------
 1 file changed, 43 insertions(+), 33 deletions(-)

diff --git a/fractal_tasks_core/lib_channels.py b/fractal_tasks_core/lib_channels.py
index 424a94bf6..b18cf0dec 100644
--- a/fractal_tasks_core/lib_channels.py
+++ b/fractal_tasks_core/lib_channels.py
@@ -388,57 +388,67 @@ def update_omero_channels(
     """
     new_channels = deepcopy(old_channels)
     existing_wavelength_ids: list[str] = []
+    handled_channels = []
 
-    # Step 1: handle channels that already have a "label" attribute
+    # Channels that with "wavelength_id"
     for ind, old_channel in enumerate(old_channels):
+        if "wavelength_id" in old_channel.keys():
+            handled_channels.append(ind)
+            existing_wavelength_ids.append(old_channel["wavelength_id"])
+            new_channel = old_channel.copy()
+            try:
+                label = old_channel["label"]
+            except KeyError:
+                label = str(ind + 1)
+            new_channel["label"] = label
+            new_channels[ind] = new_channel
+
+    # Channels with "label" but without "wavelength_id"
+    for ind, old_channel in enumerate(old_channels):
+        if ind in handled_channels:
+            continue
         if "label" not in old_channel.keys():
             continue
+        handled_channels.append(ind)
         label = old_channel["label"]
-        try:
-            wavelength_id = old_channel["wavelength_id"]
-        except KeyError:
-            wavelength_id = _get_new_unique_value(
-                label,
-                existing_wavelength_ids,
-            )
-            existing_wavelength_ids.append(wavelength_id)
+        wavelength_id = _get_new_unique_value(
+            label,
+            existing_wavelength_ids,
+        )
+        existing_wavelength_ids.append(wavelength_id)
         new_channel = old_channel.copy()
         new_channel["wavelength_id"] = wavelength_id
         new_channels[ind] = new_channel
 
-    # Step 2: handle channels that do not have a "label" attribute
+    # Channels without "label" and without "wavelength_id"
+    # NOTE: these channels must be treated last, as they have lower priority
+    # w.r.t. existing "wavelength_id" or "label" values
     for ind, old_channel in enumerate(old_channels):
-        if "label" in old_channel.keys():
+        if ind in handled_channels:
             continue
         label = str(ind + 1)
-        try:
-            wavelength_id = old_channel["wavelength_id"]
-        except KeyError:
-            wavelength_id = _get_new_unique_value(
-                label,
-                existing_wavelength_ids,
-            )
+        wavelength_id = _get_new_unique_value(
+            label,
+            existing_wavelength_ids,
+        )
         existing_wavelength_ids.append(wavelength_id)
         new_channel = old_channel.copy()
         new_channel["label"] = label
         new_channel["wavelength_id"] = wavelength_id
         new_channels[ind] = new_channel
 
-    # Step 3: log all label/wavelength_id additions
+    # Step 4: log all label/wavelength_id additions
     for ind, old_channel in enumerate(old_channels):
-        try:
-            label = old_channel["label"]
-            wavelength_id = new_channels[ind]["wavelength_id"]
-            logging.info(
-                f"Omero channel has {label=}; "
-                f"new attributes: {wavelength_id=}."
-            )
-        except KeyError:
-            label = new_channels[ind]["label"]
-            wavelength_id = new_channels[ind]["wavelength_id"]
-            logging.info(
-                "Omero channel has no label; "
-                f"new attributes: {label=}, {wavelength_id=}."
-            )
+        label = old_channel.get("label")
+        wavelength_id = old_channel.get("wavelength_id")
+        old_attributes = f"Old attributes: {label=}, {wavelength_id=}"
+        label = new_channels[ind]["label"]
+        wavelength_id = new_channels[ind]["wavelength_id"]
+        new_attributes = f"New attributes: {label=}, {wavelength_id=}"
+        logging.info(
+            "Omero channel update:\n"
+            f"    {old_attributes}\n"
+            f"    {new_attributes}"
+        )
 
     return new_channels

From 63258dd87b95d3a937fc816f74c5f725b70abdff Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 12:40:50 +0200
Subject: [PATCH 17/30] Fix typo

---
 fractal_tasks_core/tasks/import_ome_zarr.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fractal_tasks_core/tasks/import_ome_zarr.py b/fractal_tasks_core/tasks/import_ome_zarr.py
index f150831ba..bb461501d 100644
--- a/fractal_tasks_core/tasks/import_ome_zarr.py
+++ b/fractal_tasks_core/tasks/import_ome_zarr.py
@@ -132,7 +132,7 @@ def _process_single_image(
             if len(old_channels) != num_channels_zarr:
                 error_msg = (
                     "Channels-number mismatch: Number of channels in the "
-                    f"zarr array ({num_channels_zarr}) differ from number "
+                    f"zarr array ({num_channels_zarr}) differs from number "
                     "of channels listed in NGFF omero metadata "
                     f"({len(old_channels)})."
                 )

From 60b4afe946218cd17dbaa724220c40a2c11f5fac Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 12:41:02 +0200
Subject: [PATCH 18/30] Introduce test_import_ome_zarr_image_wrong_channels

---
 tests/tasks/test_import_ome_zarr.py | 39 ++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/tests/tasks/test_import_ome_zarr.py b/tests/tasks/test_import_ome_zarr.py
index fc51e61d0..8bb6d0144 100644
--- a/tests/tasks/test_import_ome_zarr.py
+++ b/tests/tasks/test_import_ome_zarr.py
@@ -151,7 +151,44 @@ def test_import_ome_zarr_image(
         assert (
             g.attrs["omero"]["channels"][0]["wavelength_id"]
             == EXPECTED_WAVELENGTH_ID
-        )  # noqa
+        )
+
+
+def test_import_ome_zarr_image_wrong_channels(
+    tmp_path, zenodo_zarr, zenodo_zarr_metadata
+):
+
+    # Prepare an on-disk OME-Zarr at the plate level
+    root_path = tmp_path
+    prepare_3D_zarr(
+        root_path,
+        zenodo_zarr,
+        zenodo_zarr_metadata,
+        remove_tables=True,
+        remove_omero=True,
+    )
+    zarr_name = "plate.zarr/B/03/0"
+
+    # Modify NGFF omero metadata, adding two channels (even if the Zarr array
+    # has only one)
+    g = zarr.open_group(str(root_path / zarr_name), mode="r+")
+    new_omero = dict(
+        channels=[
+            dict(color="asd"),
+            dict(color="asd"),
+        ]
+    )
+    g.attrs.update(omero=new_omero)
+
+    with pytest.raises(ValueError) as e:
+        import_ome_zarr(
+            input_paths=[str(root_path)],
+            zarr_name=zarr_name,
+            output_path="null",
+            metadata={},
+        )
+    debug(e.value)
+    assert "Channels-number mismatch" in str(e.value)
 
 
 @pytest.mark.skip

From 07555b1a15212ce9d1920edf291e7f81d216cfae Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 12:41:31 +0200
Subject: [PATCH 19/30] Minor update to
 test_import_ome_zarr_image_wrong_channels [skip ci]

---
 tests/tasks/test_import_ome_zarr.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/tasks/test_import_ome_zarr.py b/tests/tasks/test_import_ome_zarr.py
index 8bb6d0144..cbf4c55a5 100644
--- a/tests/tasks/test_import_ome_zarr.py
+++ b/tests/tasks/test_import_ome_zarr.py
@@ -157,7 +157,6 @@ def test_import_ome_zarr_image(
 def test_import_ome_zarr_image_wrong_channels(
     tmp_path, zenodo_zarr, zenodo_zarr_metadata
 ):
-
     # Prepare an on-disk OME-Zarr at the plate level
     root_path = tmp_path
     prepare_3D_zarr(
@@ -168,7 +167,6 @@ def test_import_ome_zarr_image_wrong_channels(
         remove_omero=True,
     )
     zarr_name = "plate.zarr/B/03/0"
-
     # Modify NGFF omero metadata, adding two channels (even if the Zarr array
     # has only one)
     g = zarr.open_group(str(root_path / zarr_name), mode="r+")
@@ -179,7 +177,7 @@ def test_import_ome_zarr_image_wrong_channels(
         ]
     )
     g.attrs.update(omero=new_omero)
-
+    # Run import_ome_zarr and catch the error
     with pytest.raises(ValueError) as e:
         import_ome_zarr(
             input_paths=[str(root_path)],

From eecb7b9756801727dd663a42024f461e8e6ccbc9 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 12:42:48 +0200
Subject: [PATCH 20/30] Add logs

---
 fractal_tasks_core/tasks/import_ome_zarr.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fractal_tasks_core/tasks/import_ome_zarr.py b/fractal_tasks_core/tasks/import_ome_zarr.py
index bb461501d..1e27c5279 100644
--- a/fractal_tasks_core/tasks/import_ome_zarr.py
+++ b/fractal_tasks_core/tasks/import_ome_zarr.py
@@ -117,6 +117,8 @@ def _process_single_image(
                 f"Existing axes ({image_meta.axes_names}) do not "
                 'include "c".'
             )
+        logger.info(f"Existing axes: {image_meta.axes_names}")
+        logger.info(f"Channel-axis index: {channel_axis_index}")
         num_channels_zarr = array.shape[channel_axis_index]
         logger.info(
             f"{num_channels_zarr} channel(s) found in Zarr array "

From a8fbe7379ff78ea2f5ec75ac934a355db17377b2 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 12:44:20 +0200
Subject: [PATCH 21/30] Remove blank line [skip ci]

---
 fractal_tasks_core/tasks/import_ome_zarr.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fractal_tasks_core/tasks/import_ome_zarr.py b/fractal_tasks_core/tasks/import_ome_zarr.py
index 1e27c5279..56870cfdb 100644
--- a/fractal_tasks_core/tasks/import_ome_zarr.py
+++ b/fractal_tasks_core/tasks/import_ome_zarr.py
@@ -108,7 +108,6 @@ def _process_single_image(
 
     # Update Omero-channels metadata
     if update_omero_metadata:
-
         # Extract number of channels from zarr array
         try:
             channel_axis_index = image_meta.axes_names.index("c")

From 2ba59a1cb70799e4a282bec40b7c94efe520e681 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 12:45:54 +0200
Subject: [PATCH 22/30] Improve comments [skip ci]

---
 fractal_tasks_core/tasks/import_ome_zarr.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fractal_tasks_core/tasks/import_ome_zarr.py b/fractal_tasks_core/tasks/import_ome_zarr.py
index 56870cfdb..79f3762cb 100644
--- a/fractal_tasks_core/tasks/import_ome_zarr.py
+++ b/fractal_tasks_core/tasks/import_ome_zarr.py
@@ -123,13 +123,13 @@ def _process_single_image(
             f"{num_channels_zarr} channel(s) found in Zarr array "
             f"at {image_path}/{dataset_subpath}"
         )
+        # Update or create omero channels metadata
         old_omero = image_group.attrs.get("omero", {})
         old_channels = old_omero.get("channels", [])
         if len(old_channels) > 0:
             logger.info(
                 f"{len(old_channels)} channel(s) found in NGFF omero metadata"
             )
-            # Case 1: omero/channels exist
             if len(old_channels) != num_channels_zarr:
                 error_msg = (
                     "Channels-number mismatch: Number of channels in the "
@@ -140,7 +140,6 @@ def _process_single_image(
                 logging.error(error_msg)
                 raise ValueError(error_msg)
         else:
-            # Case 2: omero or omero/channels do not exist
             old_channels = [{} for ind in range(num_channels_zarr)]
         new_channels = update_omero_channels(old_channels)
         new_omero = old_omero.copy()

From e399b96961270e72bfbf2ab4ab5fbf997bd88c87 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 15:20:33 +0200
Subject: [PATCH 23/30] Always add missin `color` omero-channel-metadata
 attribute, when missing (ref #158)

---
 fractal_tasks_core/lib_channels.py | 24 ++++++++++++++++++++++--
 tests/test_unit_channels.py        | 16 ++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/fractal_tasks_core/lib_channels.py b/fractal_tasks_core/lib_channels.py
index b18cf0dec..a52e8ac67 100644
--- a/fractal_tasks_core/lib_channels.py
+++ b/fractal_tasks_core/lib_channels.py
@@ -390,6 +390,14 @@ def update_omero_channels(
     existing_wavelength_ids: list[str] = []
     handled_channels = []
 
+    default_colors = ["00FFFF", "FF00FF", "FFFF00"]
+
+    def _get_next_color() -> str:
+        try:
+            return default_colors.pop(0)
+        except IndexError:
+            return "808080"
+
     # Channels that with "wavelength_id"
     for ind, old_channel in enumerate(old_channels):
         if "wavelength_id" in old_channel.keys():
@@ -401,6 +409,8 @@ def update_omero_channels(
             except KeyError:
                 label = str(ind + 1)
             new_channel["label"] = label
+            if "color" not in old_channel:
+                new_channel["color"] = _get_next_color()
             new_channels[ind] = new_channel
 
     # Channels with "label" but without "wavelength_id"
@@ -418,6 +428,8 @@ def update_omero_channels(
         existing_wavelength_ids.append(wavelength_id)
         new_channel = old_channel.copy()
         new_channel["wavelength_id"] = wavelength_id
+        if "color" not in old_channel:
+            new_channel["color"] = _get_next_color()
         new_channels[ind] = new_channel
 
     # Channels without "label" and without "wavelength_id"
@@ -435,16 +447,24 @@ def update_omero_channels(
         new_channel = old_channel.copy()
         new_channel["label"] = label
         new_channel["wavelength_id"] = wavelength_id
+        if "color" not in old_channel:
+            new_channel["color"] = _get_next_color()
         new_channels[ind] = new_channel
 
     # Step 4: log all label/wavelength_id additions
     for ind, old_channel in enumerate(old_channels):
         label = old_channel.get("label")
+        color = old_channel.get("color")
         wavelength_id = old_channel.get("wavelength_id")
-        old_attributes = f"Old attributes: {label=}, {wavelength_id=}"
+        old_attributes = (
+            f"Old attributes: {label=}, {wavelength_id=}, {color=}"
+        )
         label = new_channels[ind]["label"]
         wavelength_id = new_channels[ind]["wavelength_id"]
-        new_attributes = f"New attributes: {label=}, {wavelength_id=}"
+        color = new_channels[ind]["color"]
+        new_attributes = (
+            f"New attributes: {label=}, {wavelength_id=}, {color=}"
+        )
         logging.info(
             "Omero channel update:\n"
             f"    {old_attributes}\n"
diff --git a/tests/test_unit_channels.py b/tests/test_unit_channels.py
index a9abca5bd..f3c9141c2 100644
--- a/tests/test_unit_channels.py
+++ b/tests/test_unit_channels.py
@@ -203,6 +203,8 @@ def test_color_validator():
     "old_channels",
     [
         [{}, {}, {}],
+        [{}, {}, {}, {}],
+        [{}, {}, {}, {}, {}],
         [{"label": "A"}, {"label": "B"}, {"label": "C"}],
         [{"label": "A"}, {}, {"label": "C"}],
         [{"label": "A"}, {"label": "A"}, {"label": "A"}],
@@ -214,6 +216,7 @@ def test_color_validator():
             {},
             {"label": "1", "wavelength_id": "3"},
         ],
+        [{"color": "FFFFFF"}, {}, {}, {}, {}, {}],
     ],
 )
 def test_update_omero_channels(old_channels):
@@ -229,3 +232,16 @@ def test_update_omero_channels(old_channels):
     check_unique_wavelength_ids(
         [OmeroChannel(**channel) for channel in new_channels]
     )
+
+    # Check that colors are as expected
+    old_colors = [channel.get("color") for channel in old_channels]
+    new_colors = [channel["color"] for channel in new_channels]
+    if set(old_colors) == {None}:
+        full_colors_list = ["00FFFF", "FF00FF", "FFFF00"] + ["808080"] * 20
+        EXPECTED_COLORS = full_colors_list[: len(new_colors)]
+        debug(EXPECTED_COLORS)
+        debug(new_channels)
+        # Note: we compare sets, because the list order of `new_colors` depends
+        # on other factors (namely whether each channel has the `wavelength_id`
+        # and/or `label` attributes)
+        assert set(EXPECTED_COLORS) == set(new_colors)

From dc5bb7def9279ab5b4970f75172940d5f91494fc Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 15:23:33 +0200
Subject: [PATCH 24/30] Fix test

---
 tests/tasks/test_import_ome_zarr.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/tasks/test_import_ome_zarr.py b/tests/tasks/test_import_ome_zarr.py
index cbf4c55a5..06684e766 100644
--- a/tests/tasks/test_import_ome_zarr.py
+++ b/tests/tasks/test_import_ome_zarr.py
@@ -142,7 +142,9 @@ def test_import_ome_zarr_image(
     g = zarr.open_group(str(root_path / zarr_name), mode="r")
     debug(g.attrs["omero"]["channels"])
     if reset_omero:
-        EXPECTED_CHANNELS = [dict(label="1", wavelength_id="1")]
+        EXPECTED_CHANNELS = [
+            dict(label="1", wavelength_id="1", color="00FFFF")
+        ]
         assert g.attrs["omero"]["channels"] == EXPECTED_CHANNELS
     else:
         EXPECTED_LABEL = "DAPI"

From 76b2445d301f5b2390451b012a06805db748945a Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 15:29:51 +0200
Subject: [PATCH 25/30] Update CHANGELOG [skip ci]

---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1ab25d4a5..fd7be4cec 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,7 +8,7 @@
         * Introduce `get_single_image_ROI` and `get_image_grid_ROIs` (\#557).
         * Introduce `detect_ome_ngff_type` (\#557).
         * Introduce `update_omero_channels` (\#579).
-    * Make `maximum_intensity_projection` task not depend on ROI tables (\#557).
+    * Make `maximum_intensity_projection` independent from ROI tables (\#557).
     * Make Cellpose task work when `input_ROI_table` is empty (\#566).
     * Fix bug of missing attributes in ROI-table Zarr group (\#573).
 * Dependencies:

From 76e7492a61f900111af56507d32b0a641ab4b1f5 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 15:35:08 +0200
Subject: [PATCH 26/30] Fix docstring [skip ci]

---
 fractal_tasks_core/lib_channels.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fractal_tasks_core/lib_channels.py b/fractal_tasks_core/lib_channels.py
index a52e8ac67..40f5a6586 100644
--- a/fractal_tasks_core/lib_channels.py
+++ b/fractal_tasks_core/lib_channels.py
@@ -374,8 +374,8 @@ def update_omero_channels(
     """
     Make an existing list of Omero channels Fractal-compatible
 
-    The output channels all have `label` and `wavelength_id` keys, with the
-    `wavelength_id` values being unique across the channel list.
+    The output channels all have keys `label`, `wavelength_id` and `color`;
+    the `wavelength_id` values are unique across the channel list.
 
     See https://ngff.openmicroscopy.org/0.4/index.html#omero-md for the
     definition of NGFF Omero metadata.

From 32a4ad6d5a1afdf9d61813c5e9e3e35e58d5dc4e Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 15:35:26 +0200
Subject: [PATCH 27/30] Update CHANGELOG [skip ci]

---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fd7be4cec..c4810eeaa 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,6 @@
 **Note**: Numbers like (\#123) point to closed Pull Requests on the fractal-tasks-core repository.
 
-# Unreleased
+# 0.13.0
 
 * Tasks:
     * New task and helper functions:

From f4113bb14a0765811eb7f5b1ba92fb0754fe758b Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 15:39:44 +0200
Subject: [PATCH 28/30] Improve comment [skip ci]

---
 fractal_tasks_core/lib_channels.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fractal_tasks_core/lib_channels.py b/fractal_tasks_core/lib_channels.py
index 40f5a6586..0f2ce6f48 100644
--- a/fractal_tasks_core/lib_channels.py
+++ b/fractal_tasks_core/lib_channels.py
@@ -451,7 +451,7 @@ def _get_next_color() -> str:
             new_channel["color"] = _get_next_color()
         new_channels[ind] = new_channel
 
-    # Step 4: log all label/wavelength_id additions
+    # Log old/new values of label, wavelength_id and color
     for ind, old_channel in enumerate(old_channels):
         label = old_channel.get("label")
         color = old_channel.get("color")

From 0cad3fcf9fc6fc1b2829e7953a01758d78ecb5c9 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Thu, 19 Oct 2023 15:41:03 +0200
Subject: [PATCH 29/30] Improve logs in fixture [skip ci]

---
 tests/_zenodo_ome_zarrs.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/_zenodo_ome_zarrs.py b/tests/_zenodo_ome_zarrs.py
index f73bab122..f0976879f 100644
--- a/tests/_zenodo_ome_zarrs.py
+++ b/tests/_zenodo_ome_zarrs.py
@@ -38,7 +38,7 @@ def prepare_3D_zarr(
         shutil.rmtree(
             str(Path(zarr_path) / Path(zenodo_zarr_3D).name / "B/03/0/tables")
         )
-        logging.warning("Removing ROI tables attributes from zenodo zarr")
+        logging.warning("Removing ROI tables attributes 3D Zenodo zarr")
     if remove_omero:
         image_group = zarr.open_group(
             str(Path(zarr_path) / Path(zenodo_zarr_3D).name / "B/03/0"),
@@ -47,7 +47,7 @@ def prepare_3D_zarr(
         image_attrs = image_group.attrs.asdict()
         image_attrs.pop("omero")
         image_group.attrs.put(image_attrs)
-        logging.warning("Removing omero attributes from zenodo zarr")
+        logging.warning("Removing omero attributes from 3D Zenodo zarr")
     metadata = metadata_3D.copy()
     return metadata
 

From 683bcf896e4d4ae7241213b526cdbd2cca708914 Mon Sep 17 00:00:00 2001
From: Tommaso Comparin <3862206+tcompa@users.noreply.github.com>
Date: Fri, 20 Oct 2023 08:39:33 +0200
Subject: [PATCH 30/30] Improve error message and comments

---
 fractal_tasks_core/lib_channels.py          |  7 ++++---
 fractal_tasks_core/tasks/import_ome_zarr.py | 11 ++++++++---
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/fractal_tasks_core/lib_channels.py b/fractal_tasks_core/lib_channels.py
index 0f2ce6f48..4b79487a7 100644
--- a/fractal_tasks_core/lib_channels.py
+++ b/fractal_tasks_core/lib_channels.py
@@ -398,7 +398,7 @@ def _get_next_color() -> str:
         except IndexError:
             return "808080"
 
-    # Channels that with "wavelength_id"
+    # Channels that contain the key "wavelength_id"
     for ind, old_channel in enumerate(old_channels):
         if "wavelength_id" in old_channel.keys():
             handled_channels.append(ind)
@@ -413,7 +413,8 @@ def _get_next_color() -> str:
                 new_channel["color"] = _get_next_color()
             new_channels[ind] = new_channel
 
-    # Channels with "label" but without "wavelength_id"
+    # Channels that contain the key "label" but do not contain the key
+    # "wavelength_id"
     for ind, old_channel in enumerate(old_channels):
         if ind in handled_channels:
             continue
@@ -432,7 +433,7 @@ def _get_next_color() -> str:
             new_channel["color"] = _get_next_color()
         new_channels[ind] = new_channel
 
-    # Channels without "label" and without "wavelength_id"
+    # Channels that do not contain the key "label" nor the key "wavelength_id"
     # NOTE: these channels must be treated last, as they have lower priority
     # w.r.t. existing "wavelength_id" or "label" values
     for ind, old_channel in enumerate(old_channels):
diff --git a/fractal_tasks_core/tasks/import_ome_zarr.py b/fractal_tasks_core/tasks/import_ome_zarr.py
index 79f3762cb..033ffffd3 100644
--- a/fractal_tasks_core/tasks/import_ome_zarr.py
+++ b/fractal_tasks_core/tasks/import_ome_zarr.py
@@ -112,10 +112,15 @@ def _process_single_image(
         try:
             channel_axis_index = image_meta.axes_names.index("c")
         except ValueError:
-            raise NotImplementedError(
-                f"Existing axes ({image_meta.axes_names}) do not "
-                'include "c".'
+            logger.error(f"Existing axes: {image_meta.axes_names}")
+            msg = (
+                "OME-Zarrs with no channel axis are not currently "
+                "supported in fractal-tasks-core. Upcoming flexibility "
+                "improvements are tracked in https://github.com/"
+                "fractal-analytics-platform/fractal-tasks-core/issues/150."
             )
+            logger.error(msg)
+            raise NotImplementedError(msg)
         logger.info(f"Existing axes: {image_meta.axes_names}")
         logger.info(f"Channel-axis index: {channel_axis_index}")
         num_channels_zarr = array.shape[channel_axis_index]