Skip to content

Assign distinct colors to DICOM SEG segments (ITK export path) - #1909

Open
ousamabenyounes wants to merge 2 commits into
Project-MONAI:mainfrom
ousamabenyounes:fix/issue-1751
Open

Assign distinct colors to DICOM SEG segments (ITK export path)#1909
ousamabenyounes wants to merge 2 commits into
Project-MONAI:mainfrom
ousamabenyounes:fix/issue-1751

Conversation

@ousamabenyounes

@ousamabenyounes ousamabenyounes commented Aug 10, 2026

Copy link
Copy Markdown

Summary

Fixes #1751 — DICOM SEG shows all segments as the same color (red).

The ITK/dcmqi export path used the same red fallback whenever a segment had no explicit color and its name was absent from GENERIC_ANATOMY_COLORS.

Change

  • Add get_segment_color(name, info, index) with the existing priority: explicit color, known anatomy color, then an indexed fallback palette color.
  • Exclude the black background entry and deduplicate repeated RGB tuples while preserving palette order.
  • Use the helper in _itk_nifti_to_dicom_seg and cover the first prior duplicate span with 17 unknown segments.

Behavior for explicit colors and known anatomy names is unchanged. The default highdicom path is unaffected.

Test verification (RED → GREEN)

RED — before palette deduplication, the 17-segment regression failed:

AssertionError: 16 != 17
1 failed, 4 passed

GREEN — after deduplication:

5 passed
monailabel/datastore/utils/colors.py: 100% coverage

All repository pre-commit hooks pass locally from a clean isolated cache on Python 3.12. The remote pre-commit.ci run is currently blocked by the pinned pyupgrade hook crashing inside pre-commit.ci's Python 3.14 environment (TypeError: cannot use a bytes pattern on a string-like object); black, flake8, mypy, and every other remote hook pass.

The full local unit replay is iso-baseline: upstream and this branch both stop during collection on the same pre-existing girder_client / pkg_resources dependency incompatibility; this branch collects five additional tests before that identical failure.

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 Project-MONAI#1751

Signed-off-by: Ben Younes <benyounes.ousama@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c8baff2e-b591-4e1f-bdf7-194e0829ca3f

📥 Commits

Reviewing files that changed from the base of the PR and between 99fd12b and 63bc1e5.

📒 Files selected for processing (2)
  • monailabel/datastore/utils/colors.py
  • tests/unit/datastore/test_colors.py

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


Walkthrough

The change adds centralized segment color resolution. Explicit metadata colors take precedence, known anatomy names use the anatomy palette, and unknown segments receive indexed non-background colors. ITK/dcmqi conversion now uses this helper.

Changes

Segment Color Resolution

Layer / File(s) Summary
Add and validate segment color resolution
monailabel/datastore/utils/colors.py, tests/unit/datastore/test_colors.py
Adds get_segment_color with metadata, anatomy palette, and fallback handling. Tests validate RGB truncation, distinct colors, and exclusion of red and black background colors.
Use resolved colors in conversion
monailabel/datastore/utils/convert.py
Replaces the unknown-segment red default with indexed color resolution through get_segment_color.

Estimated code review effort: 2 (Simple) | ~15 minutes

Merge Risk: ⚪ Minimal · up to 63bc1

This PR localizes the color-selection change to the ITK DICOM SEG export path and adds regression coverage; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy issue #1751 by preserving known colors and assigning distinct non-background fallback colors to unnamed DICOM SEG segments.
Out of Scope Changes check ✅ Passed The helper, ITK conversion update, and focused unit tests are directly related to the linked issue and stated PR objectives.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: assigning distinct colors to DICOM SEG segments in the ITK export path.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@monailabel/datastore/utils/colors.py`:
- Around line 332-335: Update the _FALLBACK_COLORS construction in colors.py to
remove duplicate RGB tuples while preserving their original order, so
modulo-based indexing cannot reuse colors from duplicate entries. Extend the
regression test covering fallback color selection to include the first duplicate
span, including at least 17 unknown segments.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 04c02e51-910a-43f1-986d-ff8807a57872

📥 Commits

Reviewing files that changed from the base of the PR and between 6ed8f8c and 99fd12b.

📒 Files selected for processing (3)
  • monailabel/datastore/utils/colors.py
  • monailabel/datastore/utils/convert.py
  • tests/unit/datastore/test_colors.py

Comment thread monailabel/datastore/utils/colors.py Outdated
Signed-off-by: Ousama Ben Younes <benyounes.ousama@gmail.com>
@ousamabenyounes

Copy link
Copy Markdown
Author

Thanks for the review. Addressed the duplicate fallback RGB values in 63bc1e5 by deduplicating the palette while preserving order, and expanded the regression to cover 17 unknown segments.

Validation: the expanded test reproduced RED (16 != 17) before the fix, then passed 5/5 with 100% coverage of the changed production file. Pre-commit passed. The full local suite is iso-baseline: upstream and the fixed branch both stop on the same pre-existing girder_client / pkg_resources collection error.

@ousamabenyounes

Copy link
Copy Markdown
Author

Clarification on validation: all repository pre-commit hooks pass locally from a clean isolated cache on Python 3.12. The remote pre-commit.ci status is failing because the pinned pyupgrade hook crashes inside pre-commit.ci's Python 3.14 environment with TypeError: cannot use a bytes pattern on a string-like object; black, flake8, mypy, and every other remote hook pass. No formatting changes or autofix commit were produced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dicom seg shows all labels as color red

1 participant