From 99fd12bb011e2d7a1e9b63f7a8c20c9235f4eb1f Mon Sep 17 00:00:00 2001 From: Ben Younes Date: Mon, 10 Aug 2026 21:32:42 +0000 Subject: [PATCH 1/3] Assign distinct colors to DICOM SEG segments (ITK path) The ITK/dcmqi DICOM SEG export path built each segment's color as info.get("color", GENERIC_ANATOMY_COLORS.get(name, (255, 0, 0))). When a segment had no explicit color and its name was not a known anatomy term, every segment fell back to red, so a multi-segment DICOM SEG showed up entirely red. Add a get_segment_color() helper that keeps the existing priority (explicit color > known anatomy name) but, for the remaining case, picks a distinct color from the generic-anatomy palette by segment index instead of always red. The black background entry is excluded so a segment is never invisible. Fixes #1751 Signed-off-by: Ben Younes --- monailabel/datastore/utils/colors.py | 40 +++++++++++++++++++++ monailabel/datastore/utils/convert.py | 9 ++--- tests/unit/datastore/test_colors.py | 51 +++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 tests/unit/datastore/test_colors.py diff --git a/monailabel/datastore/utils/colors.py b/monailabel/datastore/utils/colors.py index 7c4cdb5c9..2c056e1ea 100644 --- a/monailabel/datastore/utils/colors.py +++ b/monailabel/datastore/utils/colors.py @@ -321,3 +321,43 @@ "unknown": (100, 100, 130), "cyst": (205, 205, 100), } + +# Key of the black background entry, excluded from the fallback palette so that a +# segment never resolves to an invisible (0, 0, 0) color. +_BACKGROUND_KEY = "background" + +# Number of channels in an RGB color triple. +_RGB_CHANNELS = 3 + +# Distinct, non-background colors used to give segments without a known anatomy +# name (and without an explicit color) different colors instead of all defaulting +# to red. See issue #1751. +_FALLBACK_COLORS = [color for name, color in GENERIC_ANATOMY_COLORS.items() if name != _BACKGROUND_KEY] + + +def get_segment_color(name, info=None, index=0): + """Return an ``[r, g, b]`` display color for a DICOM SEG segment. + + Resolution order: + + 1. an explicit ``color`` provided in ``info`` (from the label metadata); + 2. the palette entry whose key matches ``name`` in ``GENERIC_ANATOMY_COLORS``; + 3. a palette color chosen by ``index`` so that consecutive segments without a + known anatomy name receive *distinct* colors instead of all defaulting to + red (see issue #1751). + + Args: + name: Segment label used to look up a known anatomy color. + info: Optional segment metadata; an explicit ``color`` takes precedence. + index: Position of the segment, used to pick a distinct fallback color. + + Returns: + A list of three ``int`` RGB channel values. + """ + info = info or {} + color = info.get("color") + if color: + return [int(c) for c in list(color)[:_RGB_CHANNELS]] + if name in GENERIC_ANATOMY_COLORS: + return [int(c) for c in GENERIC_ANATOMY_COLORS[name][:_RGB_CHANNELS]] + return [int(c) for c in _FALLBACK_COLORS[index % len(_FALLBACK_COLORS)][:_RGB_CHANNELS]] diff --git a/monailabel/datastore/utils/convert.py b/monailabel/datastore/utils/convert.py index b990ba86c..ee6a7aed9 100644 --- a/monailabel/datastore/utils/convert.py +++ b/monailabel/datastore/utils/convert.py @@ -35,7 +35,7 @@ from monailabel import __version__ from monailabel.config import settings -from monailabel.datastore.utils.colors import GENERIC_ANATOMY_COLORS +from monailabel.datastore.utils.colors import get_segment_color from monailabel.transform.writer import write_itk logger = logging.getLogger(__name__) @@ -434,9 +434,10 @@ def _itk_nifti_to_dicom_seg(series_dir, label, label_info) -> str: # Use custom attribute as-is segment_descriptions.append(segment_attr) else: - # Build default template for ITK method - rgb = list(info.get("color", GENERIC_ANATOMY_COLORS.get(name, (255, 0, 0))))[0:3] - rgb = [int(x) for x in rgb] + # Build default template for ITK method. Segments without an explicit + # color or a known anatomy name get a distinct fallback color (keyed on + # their index) instead of all defaulting to red (see issue #1751). + rgb = get_segment_color(name, info, i) segment_attr = { "labelID": int(label_id), diff --git a/tests/unit/datastore/test_colors.py b/tests/unit/datastore/test_colors.py new file mode 100644 index 000000000..e91d30d06 --- /dev/null +++ b/tests/unit/datastore/test_colors.py @@ -0,0 +1,51 @@ +# Copyright (c) MONAI Consortium +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +from monailabel.datastore.utils.colors import GENERIC_ANATOMY_COLORS, get_segment_color + +# Number of unnamed segments used to check that fallback colors are distinct. +_UNNAMED_SEGMENTS = 5 +# The color every unnamed segment used to collapse to before the fix (issue #1751). +_LEGACY_RED = [255, 0, 0] +# Black background color that must never be assigned to a segment. +_BLACK = [0, 0, 0] + + +class TestGetSegmentColor(unittest.TestCase): + def test_explicit_color_takes_precedence(self): + self.assertEqual(get_segment_color("anything", {"color": [10, 20, 30]}, 0), [10, 20, 30]) + + def test_explicit_color_is_truncated_to_rgb(self): + # An RGBA (or longer) color is reduced to its first three channels. + self.assertEqual(get_segment_color("anything", {"color": [10, 20, 30, 255]}, 0), [10, 20, 30]) + + def test_known_anatomy_name_uses_palette(self): + self.assertEqual(get_segment_color("bone"), list(GENERIC_ANATOMY_COLORS["bone"])) + + def test_unnamed_segments_get_distinct_colors(self): + # Regression for #1751: segments without an explicit color and without a + # known anatomy name previously all fell back to red, so a multi-segment + # DICOM SEG showed up entirely red. They must now be distinct. + colors = [get_segment_color(f"Segment_{i}", {}, i) for i in range(_UNNAMED_SEGMENTS)] + distinct = {tuple(color) for color in colors} + self.assertEqual(len(distinct), _UNNAMED_SEGMENTS) + self.assertNotIn(_LEGACY_RED, colors) + + def test_background_is_never_selected(self): + # The black background entry must never be handed to a real segment. + for i in range(len(GENERIC_ANATOMY_COLORS)): + self.assertNotEqual(get_segment_color("no-such-name", {}, i), _BLACK) + + +if __name__ == "__main__": + unittest.main() From 63bc1e5945fa0ae003f2bb705a22f54c2c10a98c Mon Sep 17 00:00:00 2001 From: Ousama Ben Younes Date: Thu, 20 Aug 2026 03:02:00 +0000 Subject: [PATCH 2/3] Deduplicate DICOM fallback colors Signed-off-by: Ousama Ben Younes --- monailabel/datastore/utils/colors.py | 4 +++- tests/unit/datastore/test_colors.py | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/monailabel/datastore/utils/colors.py b/monailabel/datastore/utils/colors.py index 2c056e1ea..1159b584b 100644 --- a/monailabel/datastore/utils/colors.py +++ b/monailabel/datastore/utils/colors.py @@ -332,7 +332,9 @@ # Distinct, non-background colors used to give segments without a known anatomy # name (and without an explicit color) different colors instead of all defaulting # to red. See issue #1751. -_FALLBACK_COLORS = [color for name, color in GENERIC_ANATOMY_COLORS.items() if name != _BACKGROUND_KEY] +_FALLBACK_COLORS = list( + dict.fromkeys(color for name, color in GENERIC_ANATOMY_COLORS.items() if name != _BACKGROUND_KEY) +) def get_segment_color(name, info=None, index=0): diff --git a/tests/unit/datastore/test_colors.py b/tests/unit/datastore/test_colors.py index e91d30d06..1c86236de 100644 --- a/tests/unit/datastore/test_colors.py +++ b/tests/unit/datastore/test_colors.py @@ -13,8 +13,9 @@ from monailabel.datastore.utils.colors import GENERIC_ANATOMY_COLORS, get_segment_color -# Number of unnamed segments used to check that fallback colors are distinct. -_UNNAMED_SEGMENTS = 5 +# Number of unnamed segments needed to cover the fallback palette's first +# duplicate RGB value, which previously appeared at indices 4 and 16. +_UNNAMED_SEGMENTS = 17 # The color every unnamed segment used to collapse to before the fix (issue #1751). _LEGACY_RED = [255, 0, 0] # Black background color that must never be assigned to a segment. From edbf4c55dea287a3113edfb688b8d93dd39f0bc0 Mon Sep 17 00:00:00 2001 From: Ousama Ben Younes Date: Sat, 24 Oct 2026 16:19:44 +0000 Subject: [PATCH 3/3] Reserve fixed segment colors before fallback assignment Unknown DICOM SEG segments picked a fallback color by index without considering colors already assigned in the same export, so an unknown segment could duplicate a fixed one. _itk_nifti_to_dicom_seg now reserves every color fixed by explicit metadata, a known anatomy name, or a custom segmentAttribute, and get_segment_color skips reserved colors when choosing a fallback. Addresses the CodeRabbit review on #1909 (issue #1751). Signed-off-by: Ousama Ben Younes --- monailabel/datastore/utils/colors.py | 52 ++++++++++++++++++++++----- monailabel/datastore/utils/convert.py | 25 +++++++++++-- tests/unit/datastore/test_colors.py | 10 ++++++ 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/monailabel/datastore/utils/colors.py b/monailabel/datastore/utils/colors.py index 1159b584b..8aee98e55 100644 --- a/monailabel/datastore/utils/colors.py +++ b/monailabel/datastore/utils/colors.py @@ -337,7 +337,32 @@ ) -def get_segment_color(name, info=None, index=0): +def fixed_segment_color(name, info=None): + """Return the *fixed* ``[r, g, b]`` color for a segment, or ``None``. + + A segment has a fixed color when its metadata carries an explicit ``color`` + or when its ``name`` matches a known anatomy entry in + ``GENERIC_ANATOMY_COLORS``. Unknown segments have no fixed color and fall + back to an index-based palette choice in :func:`get_segment_color`. + + Args: + name: Segment label used to look up a known anatomy color. + info: Optional segment metadata; an explicit ``color`` takes precedence. + + Returns: + A list of three ``int`` RGB channel values, or ``None`` when the segment + has neither an explicit color nor a known anatomy name. + """ + info = info or {} + color = info.get("color") + if color: + return [int(c) for c in list(color)[:_RGB_CHANNELS]] + if name in GENERIC_ANATOMY_COLORS: + return [int(c) for c in GENERIC_ANATOMY_COLORS[name][:_RGB_CHANNELS]] + return None + + +def get_segment_color(name, info=None, index=0, reserved=None): """Return an ``[r, g, b]`` display color for a DICOM SEG segment. Resolution order: @@ -348,18 +373,29 @@ def get_segment_color(name, info=None, index=0): known anatomy name receive *distinct* colors instead of all defaulting to red (see issue #1751). + For the fallback case (3), any color listed in ``reserved`` is skipped so an + unknown segment never duplicates a color already assigned to another segment + in the same export (CodeRabbit review on #1909). If every fallback color is + reserved, the plain index-based choice is returned as a last resort. + Args: name: Segment label used to look up a known anatomy color. info: Optional segment metadata; an explicit ``color`` takes precedence. index: Position of the segment, used to pick a distinct fallback color. + reserved: Optional iterable of ``(r, g, b)`` colors already assigned to + other segments, avoided when selecting a fallback color. Returns: A list of three ``int`` RGB channel values. """ - info = info or {} - color = info.get("color") - if color: - return [int(c) for c in list(color)[:_RGB_CHANNELS]] - if name in GENERIC_ANATOMY_COLORS: - return [int(c) for c in GENERIC_ANATOMY_COLORS[name][:_RGB_CHANNELS]] - return [int(c) for c in _FALLBACK_COLORS[index % len(_FALLBACK_COLORS)][:_RGB_CHANNELS]] + fixed = fixed_segment_color(name, info) + if fixed is not None: + return fixed + + palette_size = len(_FALLBACK_COLORS) + reserved_set = {tuple(color) for color in reserved} if reserved else set() + for offset in range(palette_size): + candidate = _FALLBACK_COLORS[(index + offset) % palette_size] + if tuple(candidate) not in reserved_set: + return [int(c) for c in candidate[:_RGB_CHANNELS]] + return [int(c) for c in _FALLBACK_COLORS[index % palette_size][:_RGB_CHANNELS]] diff --git a/monailabel/datastore/utils/convert.py b/monailabel/datastore/utils/convert.py index ee6a7aed9..a72548bce 100644 --- a/monailabel/datastore/utils/convert.py +++ b/monailabel/datastore/utils/convert.py @@ -35,7 +35,7 @@ from monailabel import __version__ from monailabel.config import settings -from monailabel.datastore.utils.colors import get_segment_color +from monailabel.datastore.utils.colors import _RGB_CHANNELS, fixed_segment_color, get_segment_color from monailabel.transform.writer import write_itk logger = logging.getLogger(__name__) @@ -417,6 +417,23 @@ def _itk_nifti_to_dicom_seg(series_dir, label, label_info) -> str: if unique_labels is None: return "" + # Reserve every color fixed by explicit metadata, a known anatomy name, or a + # custom segmentAttribute before assigning fallback colors, so an unknown + # segment never duplicates a color already given to another segment + # (CodeRabbit review on #1909 / issue #1751). + reserved_colors = set() + for label_id in unique_labels: + info = info_by_label_id.get(int(label_id), {}) + segment_attr = info.get("segmentAttribute") + if segment_attr: + rgb = segment_attr.get("recommendedDisplayRGBValue") + if rgb: + reserved_colors.add(tuple(int(c) for c in rgb[:_RGB_CHANNELS])) + continue + fixed = fixed_segment_color(info.get("name", f"Segment_{label_id}"), info) + if fixed is not None: + reserved_colors.add(tuple(fixed)) + # Build ITK segment descriptions segment_descriptions = [] for i, label_id in enumerate(unique_labels): @@ -436,8 +453,10 @@ def _itk_nifti_to_dicom_seg(series_dir, label, label_info) -> str: else: # Build default template for ITK method. Segments without an explicit # color or a known anatomy name get a distinct fallback color (keyed on - # their index) instead of all defaulting to red (see issue #1751). - rgb = get_segment_color(name, info, i) + # their index) that avoids every reserved color (see issue #1751). + rgb = get_segment_color(name, info, i, reserved=reserved_colors) + # Keep this fallback out of later unknown segments' choices too. + reserved_colors.add(tuple(rgb)) segment_attr = { "labelID": int(label_id), diff --git a/tests/unit/datastore/test_colors.py b/tests/unit/datastore/test_colors.py index 1c86236de..92c5f8f80 100644 --- a/tests/unit/datastore/test_colors.py +++ b/tests/unit/datastore/test_colors.py @@ -47,6 +47,16 @@ def test_background_is_never_selected(self): for i in range(len(GENERIC_ANATOMY_COLORS)): self.assertNotEqual(get_segment_color("no-such-name", {}, i), _BLACK) + def test_fallback_avoids_reserved_colors(self): + # CodeRabbit review on #1909: an unknown segment's fallback color must + # not duplicate a color already assigned to another segment. Reserving + # the color the unknown segment would otherwise pick must push it to a + # different, still-distinct color. + default = get_segment_color("no-such-name", {}, 0) + avoided = get_segment_color("no-such-name", {}, 0, reserved={tuple(default)}) + self.assertNotEqual(avoided, default) + self.assertNotEqual(avoided, _BLACK) + if __name__ == "__main__": unittest.main()