feat: Add saving color maps in OME tiffs#1312
Conversation
Reviewer's GuideEnable saving of default colormaps in OME-TIFFs by encoding RGB values into signed 32-bit integers, exposing this via a new method on the Image class, and injecting the resulting values into the TIFF channel metadata; also adds a compatibility import in the common_backend initializer. Sequence diagram for saving OME-TIFF with colormap metadatasequenceDiagram
participant "Image"
participant "ImageWriter.save()"
participant "OME-TIFF file"
"ImageWriter.save()"->>"Image": get_ome_colors()
"Image"-->>"ImageWriter.save()": list of signed int colors
"ImageWriter.save()"->>"OME-TIFF file": write data and metadata (including Channel.Color)
Class diagram for updated Image class and new colormap encodingclassDiagram
class Image {
+get_imagej_colors()
+get_ome_colors() list[int] // NEW
+get_colors() list[str | list[int]]
default_coloring
}
class "_rgb_to_signed_int" {
+_rgb_to_signed_int(rgb: tuple[int, int, int]) int // NEW
}
Image --> "_rgb_to_signed_int" : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Czaki
changed the title
WalkthroughAdds strict normalization and validation for channel color maps, refines Image.get_imagej_colors typing/outputs, adds Image.get_ome_colors and _rgb_to_signed_int to encode per-channel OME colors as signed 32-bit ints, writes per-channel "Color" metadata in ImageWriter.save, and expands tests (fixtures and ChannelInfoFull validation) for ImageJ/OME color handling and error cases. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test/Caller
participant W as ImageWriter.save
participant I as Image
participant F as TIFF File Writer
T->>W: save(image, path)
W->>I: get_ome_colors()
I-->>W: [int, int, ...] (per-channel signed RGB)
Note over W: attach per-channel metadata including "Color"
W->>F: write(image data + metadata)
F-->>W: write OK
W-->>T: Completed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The import-and-delete of
themein common_backend appears to be a no-op and could be removed for clarity. - Rather than checking for
color.ndim, consider also supporting plain Python lists or tuples in get_ome_colors by detecting sequence types. - Extract the
default_colorslist into a class or module constant so it can be reused and configured instead of redefining it inline.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The import-and-delete of `theme` in common_backend appears to be a no-op and could be removed for clarity.
- Rather than checking for `color.ndim`, consider also supporting plain Python lists or tuples in get_ome_colors by detecting sequence types.
- Extract the `default_colors` list into a class or module constant so it can be reused and configured instead of redefining it inline.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1312 +/- ##
===========================================
+ Coverage 93.16% 93.18% +0.02%
===========================================
Files 210 210
Lines 33287 33365 +78
===========================================
+ Hits 31011 31092 +81
+ Misses 2276 2273 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
package/PartSeg/common_backend/__init__.py (1)
15-17: Document the reason for this side-effect import.Importing a module solely for its side effects and immediately deleting the reference is unusual and can confuse maintainers. Add a comment explaining why importing
napari.utils.themeis necessary (e.g., module registration, initialization side effects).Example:
+ # Import theme module to trigger napari color system initialization from napari.utils import theme # noqa: PLC0415 del themeAdditionally, consider removing the
noqa: PLC0415directive as Ruff reports it as unused, unless you're running other linters that require it.package/PartSegImage/image.py (1)
925-949: Verify the RGB encoding and consider edge case handling.The implementation looks mostly correct, but a few concerns:
Line 940: When
coloris annp.ndarray, accessingcolor.ndimis safe thanks toChannelInfoFull.__post_init__ensuringcolor_mapis alwaysstr | np.ndarray. However, consider what happens if the ndarray hasndim == 0(scalar array) – this would skip all branches and potentially cause issues.Line 942: Converting
colorto tuple withtuple(color)works for 1D arrays, but ensure the array contains exactly 3 elements or handle cases where it might have more/fewer. The_rgb_to_signed_intfunction uses slicingrgb[:3]which is safe, but values beyond index 2 are silently ignored.RGB value range: The code assumes RGB values are in the 0-255 range. If they're normalized (0.0-1.0) or out of range, the bit-shifting in
_rgb_to_signed_intwill produce incorrect results. Consider adding validation or normalization.Consider adding a guard for scalar arrays and RGB value validation:
def get_ome_colors(self) -> list[int]: res = [] default_colors = ["red", "blue", "green", "yellow", "magenta", "cyan"] for i, color in enumerate(self.default_coloring): if isinstance(color, str): if color.startswith("#"): color_array = _hex_to_rgb(color) else: color_array = _name_to_rgb(color) res.append(_rgb_to_signed_int(color_array)) - elif color.ndim == 1: + elif color.ndim == 1 and len(color) >= 3: # treat as RGB res.append(_rgb_to_signed_int(tuple(color))) else: logging.warning( "Do not support custom colormap in ome colors. Use %s", default_colors[i % len(default_colors)] ) color_array = _name_to_rgb(default_colors[i % len(default_colors)]) res.append(_rgb_to_signed_int(color_array)) return res
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package/PartSeg/common_backend/__init__.py(1 hunks)package/PartSegImage/image.py(3 hunks)package/PartSegImage/image_writer.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
package/PartSegImage/image_writer.py (1)
package/PartSegImage/image.py (1)
get_ome_colors(925-949)
package/PartSeg/common_backend/__init__.py (1)
package/PartSeg/common_backend/base_settings.py (1)
theme(274-284)
🪛 Ruff (0.13.3)
package/PartSeg/common_backend/__init__.py
15-15: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
🔇 Additional comments (2)
package/PartSegImage/image_writer.py (1)
76-80: LGTM! Clean integration of color metadata.The addition of the "Color" field to the Channel metadata properly integrates the new
get_ome_colors()method. The returned list of integers will be correctly serialized into the OME TIFF metadata.package/PartSegImage/image.py (1)
1031-1034: Approve code changes — encoding matches OME-TIFF RGBA signed 32-bit specification.
Czaki
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package/tests/test_PartSegImage/test_image_writer.py (1)
131-139: Verify whether ranges should be tested for OME format.The ImageJ test (line 124) verifies that ranges are correctly saved and read back, but this OME test does not include a similar check. If the OME format should preserve ranges, consider adding:
def test_ome_save_color(tmp_path, data_to_save): ImageWriter.save(data_to_save, tmp_path / "image.tif") image2 = TiffImageReader.read_image(tmp_path / "image.tif") assert image2.channel_names == ["ch1", "ch2", "ch3", "ch4"] + npt.assert_array_equal(image2.ranges, [(0, 20), (0, 30), (0, 25), (0, 22)]) assert tuple(image2.default_coloring[0]) == (0, 0, 255) assert tuple(image2.default_coloring[1]) == (255, 170, 0) assert tuple(image2.default_coloring[2]) == (255, 187, 17) assert tuple(image2.default_coloring[3]) == (0, 180, 0)
Optional: Consider parameterizing the color verification tests.
The ImageJ and OME tests share similar structure and could be parameterized to reduce duplication:
@pytest.mark.parametrize("writer_class,color_format", [ (IMAGEJImageWriter, "lut"), # Returns LUTs (256x3 arrays) (ImageWriter, "rgb"), # Returns RGB tuples ]) def test_save_color(tmp_path, data_to_save, writer_class, color_format): writer_class.save(data_to_save, tmp_path / "image.tif") image2 = TiffImageReader.read_image(tmp_path / "image.tif") assert image2.channel_names == ["ch1", "ch2", "ch3", "ch4"] npt.assert_array_equal(image2.ranges, [(0, 20), (0, 30), (0, 25), (0, 22)]) if color_format == "lut": # Verify LUT colors (last column) assert tuple(image2.default_coloring[0][:, -1]) == (0, 0, 255) # ... other channels else: # Verify RGB tuple colors assert tuple(image2.default_coloring[0]) == (0, 0, 255) # ... other channels
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package/tests/test_PartSegImage/test_image_writer.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
package/tests/test_PartSegImage/test_image_writer.py (3)
package/PartSegImage/image_writer.py (5)
IMAGEJImageWriter(116-175)save(16-17)save(65-81)save(120-140)ImageWriter(25-113)package/PartSegImage/image_reader.py (3)
TiffImageReader(460-653)read_image(167-188)read_image(267-288)package/PartSegImage/image.py (3)
channel_names(417-418)ranges(378-379)default_coloring(382-383)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: repo py3.10
- GitHub Check: repo py3.12
- GitHub Check: repo py3.11
- GitHub Check: ubuntu-24.04 py3.12 --pre
- GitHub Check: Base py3.11 / macos-15-intel py 3.11 latest PyQt5
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
🔇 Additional comments (2)
package/tests/test_PartSegImage/test_image_writer.py (2)
115-116: Verify that assertions in fixture are intentional.Fixtures typically don't contain assertions unless they're validating setup preconditions. If these assertions fail, the fixture itself fails, which may cause confusing test failures. Consider whether these assertions should be:
- Moved to the tests that use the fixture, or
- Kept here if they're truly setup validation
120-129: LGTM!The refactor to use the fixture and numpy testing assertion improves the test structure and makes the assertion more robust for array comparisons.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package/PartSegImage/image.py (1)
912-923: Critical: Fix duplicate color append in 2D colormap handling.Line 922 executes unconditionally after the interpolation block, causing colors to be appended twice when
color.shape[1] != 256. This results in duplicate entries in the returned color list.Apply this diff to fix the logic:
else: if color.shape[1] != 256: res.append( np.array( [ np.interp(np.linspace(0, 255, num=256), np.linspace(0, color.shape[1], num=256), x) for x in color ] - ) + ).astype(np.uint8) ) - res.append(color.astype(np.uint8)) + else: + res.append(color.astype(np.uint8)) return res
🧹 Nitpick comments (1)
package/PartSegImage/image.py (1)
1031-1034: Minor type hint inconsistency.The type hint specifies
tuple[int, int, int]but the implementation usesrgb[:3], suggesting it accepts tuples with more than 3 elements. Consider either:
- Updating the type hint to
tuple[int, ...]orSequence[int]to match the implementation- Removing the slicing and using direct unpacking:
r, g, b = rgbOption 1 (type hint adjustment):
-def _rgb_to_signed_int(rgb: tuple[int, int, int]) -> int: +def _rgb_to_signed_int(rgb: tuple[int, int, int] | tuple[int, ...]) -> int: """Convert an RGB tuple to a signed integer representation.""" r, g, b = rgb[:3] return np.uint32((r << 24) | (g << 16) | (b << 8) | 255).view(np.int32).item()Option 2 (implementation adjustment):
def _rgb_to_signed_int(rgb: tuple[int, int, int]) -> int: """Convert an RGB tuple to a signed integer representation.""" - r, g, b = rgb[:3] + r, g, b = rgb return np.uint32((r << 24) | (g << 16) | (b << 8) | 255).view(np.int32).item()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package/PartSegImage/image.py(3 hunks)package/tests/test_PartSegImage/test_image_writer.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
package/tests/test_PartSegImage/test_image_writer.py (3)
package/PartSegImage/image.py (6)
dtype(447-449)ChannelInfo(35-38)get_colors(951-960)channel_names(417-418)ranges(378-379)default_coloring(382-383)package/PartSegImage/image_writer.py (5)
IMAGEJImageWriter(116-175)save(16-17)save(65-81)save(120-140)ImageWriter(25-113)package/PartSegImage/image_reader.py (2)
read_image(167-188)read_image(267-288)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: repo py3.10
- GitHub Check: repo py3.12
- GitHub Check: repo py3.11
- GitHub Check: Base py3.10 / ubuntu-22.04 py 3.10 latest PySide2
- GitHub Check: Base py3.11 / ubuntu-24.04 py 3.11 latest PyQt5
- GitHub Check: test_coverage / ubuntu-24.04 py 3.12 latest PyQt5
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: Sourcery review
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
package/PartSegImage/image.py (1)
3-3: LGTM!The logging import is appropriately added to support the warning in
get_ome_colors.package/tests/test_PartSegImage/test_image_writer.py (3)
100-120: LGTM!The
data_to_savefixture provides comprehensive coverage of different color format inputs (named colors, hex codes, RGB tuples, and colormaps). The validation assertions at lines 118-119 confirm correct color parsing before the fixture returns.
123-132: LGTM!Good refactoring to use the shared
data_to_savefixture and switching tonpt.assert_array_equalfor comparing ranges provides better assertion output on failure.
135-143: Verify integer color encoding/decoding
The test covers OME color round-trip, but I couldn’t locate_rgb_to_signed_intor its correspondingdecodemethod—please manually confirm that the integer encoding matches the reader’s decoding implementation.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package/PartSegImage/image.py(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T10:23:14.634Z
Learnt from: Czaki
PR: 4DNucleome/PartSeg#1312
File: package/PartSegImage/image.py:925-949
Timestamp: 2025-10-11T10:23:14.634Z
Learning: In package/PartSegImage/image.py, the ChannelInfoFull dataclass's __post_init__ method automatically converts any color_map that is not a string or numpy.ndarray to a numpy array, ensuring that self.default_coloring (which returns [x.color_map for x in self._channel_info]) only contains strings or numpy arrays, never raw tuples or lists.
Applied to files:
package/PartSegImage/image.py
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: macos-14 py3.12 --pre
- GitHub Check: windows-2022 py3.12 --pre
- GitHub Check: ubuntu-24.04 py3.12 --pre
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: Base py3.10 / ubuntu-24.04 py 3.10 latest PyQt5
- GitHub Check: Base py3.11 / macos-15-intel py 3.11 latest PyQt5
- GitHub Check: test_coverage / ubuntu-24.04 py 3.12 latest PyQt5
- GitHub Check: repo py3.12
- GitHub Check: repo py3.10
- GitHub Check: repo py3.11
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
package/tests/test_PartSegImage/test_image.py (1)
736-737: Annotate class attributes withClassVar.Static analysis correctly flags that mutable class attributes should be annotated with
typing.ClassVarto indicate they are class-level variables, not instance attributes.Based on static analysis hints.
Apply this diff:
+from typing import ClassVar + class TestChannelInfoFull: - RED_DEF = (255, 0, 0) - RED_2D_REF = [[0, 255], [0, 0], [0, 0]] + RED_DEF: ClassVar = (255, 0, 0) + RED_2D_REF: ClassVar = [[0, 255], [0, 0], [0, 0]]
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package/PartSegImage/image.py(4 hunks)package/tests/test_PartSegImage/test_image.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T10:23:14.634Z
Learnt from: Czaki
PR: 4DNucleome/PartSeg#1312
File: package/PartSegImage/image.py:925-949
Timestamp: 2025-10-11T10:23:14.634Z
Learning: In package/PartSegImage/image.py, the ChannelInfoFull dataclass's __post_init__ method automatically converts any color_map that is not a string or numpy.ndarray to a numpy array, ensuring that self.default_coloring (which returns [x.color_map for x in self._channel_info]) only contains strings or numpy arrays, never raw tuples or lists.
Applied to files:
package/PartSegImage/image.py
🧬 Code graph analysis (1)
package/tests/test_PartSegImage/test_image.py (1)
package/PartSegImage/image.py (5)
ChannelInfo(35-38)ChannelInfoFull(42-60)Image(195-1009)dtype(458-460)shape(651-653)
🪛 GitHub Actions: --pre Test
package/PartSegImage/image.py
[error] 53-57: Colormap as array need to be uint8, not float64
[error] 53-57: Color map need to have 3 or 4 elements (RGB or RGBA), not (256,)
🪛 GitHub Actions: Tests
package/PartSegImage/image.py
[error] 53-53: Colormap as array need to be uint8, not float64
[error] 57-57: Color map need to have 3 or 4 elements (RGB or RGBA), not (256,)
package/tests/test_PartSegImage/test_image.py
[error] 1-1: PyTest run failed with exit code 1 during tox py312-PySide2-conda.
🪛 Ruff (0.14.0)
package/tests/test_PartSegImage/test_image.py
736-736: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: repo py3.10
- GitHub Check: repo py3.11
- GitHub Check: repo py3.12
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: Sourcery review
| def __post_init__(self): | ||
| if not isinstance(self.color_map, (str, np.ndarray)): | ||
| self.color_map = np.array(self.color_map) | ||
| self.color_map = np.array(self.color_map, dtype=np.uint8) | ||
| if isinstance(self.color_map, np.ndarray): | ||
| if self.color_map.dtype != np.uint8: | ||
| message = f"Colormap as array need to be uint8, not {self.color_map.dtype}" | ||
| raise ValueError(message) | ||
| if self.color_map.ndim in {1, 2}: | ||
| if self.color_map.shape[0] not in {3, 4}: | ||
| message = f"Color map need to have 3 or 4 elements (RGB or RGBA), not {self.color_map.shape}" | ||
| raise ValueError(message) | ||
| else: | ||
| message = f"Colormap as sequence need to be 1d or 2d array, not {self.color_map.shape}" | ||
| raise ValueError(message) |
There was a problem hiding this comment.
❓ Verification inconclusive
Convert numpy arrays to uint8 to prevent validation errors.
The current logic (line 49) only converts non-ndarray inputs to uint8. If a numpy array with dtype=float64 or incorrect shape is passed directly, it fails validation at lines 52-57 rather than being converted. The pipeline failures confirm this:
- "Colormap as array need to be uint8, not float64" (line 53)
- "Color map need to have 3 or 4 elements (RGB or RGBA), not (256,)" (line 57)
Consider converting all numpy arrays to the expected format or clearly document that callers must provide correctly-formatted arrays.
Apply this diff to convert numpy arrays:
def __post_init__(self):
- if not isinstance(self.color_map, (str, np.ndarray)):
- self.color_map = np.array(self.color_map, dtype=np.uint8)
- if isinstance(self.color_map, np.ndarray):
- if self.color_map.dtype != np.uint8:
- message = f"Colormap as array need to be uint8, not {self.color_map.dtype}"
- raise ValueError(message)
- if self.color_map.ndim in {1, 2}:
- if self.color_map.shape[0] not in {3, 4}:
- message = f"Color map need to have 3 or 4 elements (RGB or RGBA), not {self.color_map.shape}"
- raise ValueError(message)
- else:
- message = f"Colormap as sequence need to be 1d or 2d array, not {self.color_map.shape}"
- raise ValueError(message)
+ if not isinstance(self.color_map, str):
+ # Convert all non-string inputs to numpy array
+ if not isinstance(self.color_map, np.ndarray):
+ self.color_map = np.array(self.color_map, dtype=np.uint8)
+ else:
+ # Convert existing arrays to uint8
+ self.color_map = self.color_map.astype(np.uint8)
+
+ # Validate shape
+ if self.color_map.ndim in {1, 2}:
+ if self.color_map.shape[0] not in {3, 4}:
+ message = f"Color map need to have 3 or 4 elements (RGB or RGBA), not {self.color_map.shape}"
+ raise ValueError(message)
+ else:
+ message = f"Colormap as sequence need to be 1d or 2d array, not {self.color_map.shape}"
+ raise ValueError(message)Alternatively, document the strict validation requirement in the docstring or class documentation.
Convert existing numpy array colormaps to uint8 before validation
In package/PartSegImage/image.py:47-60, the logic only casts non-ndarray inputs to uint8, causing float64 arrays to fail. Add a .astype(np.uint8) for ndarray inputs before checking dtype and shape.
if not isinstance(self.color_map, str):
- if not isinstance(self.color_map, np.ndarray):
- self.color_map = np.array(self.color_map, dtype=np.uint8)
- else:
- # Convert existing arrays to uint8
- self.color_map = self.color_map.astype(np.uint8)
+ # Convert all non-string inputs to uint8 arrays
+ if isinstance(self.color_map, np.ndarray):
+ self.color_map = self.color_map.astype(np.uint8)
+ else:
+ self.color_map = np.array(self.color_map, dtype=np.uint8)
# Validate shape
if self.color_map.ndim in {1, 2}:
…🧰 Tools
🪛 GitHub Actions: --pre Test
[error] 53-57: Colormap as array need to be uint8, not float64
[error] 53-57: Color map need to have 3 or 4 elements (RGB or RGBA), not (256,)
🪛 GitHub Actions: Tests
[error] 53-53: Colormap as array need to be uint8, not float64
[error] 57-57: Color map need to have 3 or 4 elements (RGB or RGBA), not (256,)
🤖 Prompt for AI Agents
In package/PartSegImage/image.py around lines 47-60, the code currently only
casts non-ndarray inputs to uint8 which causes existing numpy arrays with float
dtype to fail validation; change the ndarray branch to first convert the array
to uint8 (e.g. self.color_map = self.color_map.astype(np.uint8)) before
performing the dtype and shape checks so float arrays are normalized to uint8
prior to validation.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
package/PartSegImage/image.py (3)
1042-1045: Explicitly cast bit-shift operands to Python int.Based on past review comments, if
rgbcontains numpy scalars from array conversions, the bit-shift operations may raiseTypeError. While usingnp.int32()helps, explicitly converting the input values ensures compatibility.Apply this diff for defensive casting:
def _rgb_to_signed_int(rgb: tuple[int, int, int]) -> int: """Convert an RGB tuple to a signed integer representation.""" - r, g, b = rgb[:3] - return (np.int32(r) << 24) | (np.int32(g) << 16) | (np.int32(b) << 8) | 255 + # Convert to Python ints to handle numpy scalars + r, g, b = (int(x) for x in rgb[:3]) + return int((np.int32(r) << 24) | (np.int32(g) << 16) | (np.int32(b) << 8) | 255)Based on past review comments.
936-960: Convert numpy array values to Python ints before bit-shifting.Based on past review comments and retrieved learnings, line 953's
tuple(color)produces numpy scalars (e.g.,np.uint8(255)) that may causeTypeErrorduring bit-shifting in_rgb_to_signed_int. Additionally, the code doesn't validate that 1D arrays have at least 3 elements, risking unpacking errors.Apply this diff to ensure safe conversion:
elif color.ndim == 1: # treat as RGB - res.append(_rgb_to_signed_int(tuple(color))) + if len(color) < 3: + logging.warning( + "1D color array for channel %d has only %d components, expected 3 (RGB). Using fallback color %s", + i, len(color), default_colors[i % len(default_colors)] + ) + color_array = _name_to_rgb(default_colors[i % len(default_colors)]) + res.append(_rgb_to_signed_int(color_array)) + else: + # Convert to Python ints to handle numpy scalars + res.append(_rgb_to_signed_int(tuple(int(x) for x in color[:3])))Based on past review comments.
47-60: Convert existing numpy arrays to uint8 before validation.Based on past review comments, the current validation logic still raises
ValueErrorwhen a numpy array with non-uint8 dtype (e.g.,float64) is passed directly. Line 51-53 checks the dtype but doesn't convert it first, causing pipeline failures.Apply this diff to normalize arrays to uint8 before validation:
def __post_init__(self): - if not isinstance(self.color_map, (str, np.ndarray)): + if not isinstance(self.color_map, str): + # Convert all non-string inputs to numpy array + if not isinstance(self.color_map, np.ndarray): + self.color_map = np.array(self.color_map, dtype=np.uint8) + else: + # Convert existing arrays to uint8 + self.color_map = self.color_map.astype(np.uint8) + + # Validate shape + if self.color_map.ndim in {1, 2}: + if self.color_map.shape[0] not in {3, 4}: + message = f"Color map need to have 3 or 4 elements (RGB or RGBA), not {self.color_map.shape}" + raise ValueError(message) + else: + message = f"Colormap as sequence need to be 1d or 2d array, not {self.color_map.shape}" + raise ValueError(message) - self.color_map = np.array(self.color_map, dtype=np.uint8) - if isinstance(self.color_map, np.ndarray): - if self.color_map.dtype != np.uint8: - message = f"Colormap as array need to be uint8, not {self.color_map.dtype}" - raise ValueError(message) - if self.color_map.ndim in {1, 2}: - if self.color_map.shape[0] not in {3, 4}: - message = f"Color map need to have 3 or 4 elements (RGB or RGBA), not {self.color_map.shape}" - raise ValueError(message) - else: - message = f"Colormap as sequence need to be 1d or 2d array, not {self.color_map.shape}" - raise ValueError(message)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package/PartSegImage/image.py(4 hunks)package/PartSegImage/image_reader.py(2 hunks)package/tests/test_PartSegImage/test_image_writer.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T10:23:14.634Z
Learnt from: Czaki
PR: 4DNucleome/PartSeg#1312
File: package/PartSegImage/image.py:925-949
Timestamp: 2025-10-11T10:23:14.634Z
Learning: In package/PartSegImage/image.py, the ChannelInfoFull dataclass's __post_init__ method automatically converts any color_map that is not a string or numpy.ndarray to a numpy array, ensuring that self.default_coloring (which returns [x.color_map for x in self._channel_info]) only contains strings or numpy arrays, never raw tuples or lists.
Applied to files:
package/PartSegImage/image.py
🧬 Code graph analysis (2)
package/tests/test_PartSegImage/test_image_writer.py (3)
package/PartSegImage/image.py (6)
dtype(458-460)ChannelInfo(35-38)get_colors(962-971)channel_names(428-429)ranges(389-390)default_coloring(393-394)package/PartSegImage/image_writer.py (5)
IMAGEJImageWriter(116-175)save(16-17)save(65-81)save(120-140)ImageWriter(25-113)package/PartSegImage/image_reader.py (3)
TiffImageReader(467-660)read_image(174-195)read_image(274-295)
package/PartSegImage/image_reader.py (1)
package/PartSegImage/image.py (3)
ranges(389-390)ChannelInfo(35-38)channel_names(428-429)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: Base py3.12 / ubuntu-24.04 py 3.12 latest PyQt5
- GitHub Check: Base py3.10 / ubuntu-22.04 py 3.10 latest PyQt5 _pydantic_1
- GitHub Check: Base py3.10 / ubuntu-24.04 py 3.10 latest PyQt5
- GitHub Check: Base py3.11 / macos-15-intel py 3.11 latest PyQt5
- GitHub Check: repo py3.10
- GitHub Check: repo py3.12
- GitHub Check: repo py3.11
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
🔇 Additional comments (3)
package/tests/test_PartSegImage/test_image_writer.py (3)
100-123: LGTM!The
data_to_savefixture correctly constructs a 5-channel image with various color formats (named, hex short/long, RGB tuple, and 2D colormap), providing comprehensive test coverage for the new OME color encoding functionality.
125-135: LGTM!The test correctly verifies ImageJ color saving for 5 channels, including the new 2D colormap case. The assertions appropriately validate channel names, ranges, and the final color values in the colormaps.
137-146: LGTM!The new test appropriately verifies OME color metadata encoding. Line 145's expectation of magenta
(255, 0, 255)as the fallback for 2D colormaps aligns with theget_ome_colorsimplementation, which logs a warning and uses a default color when encountering unsupported colormap formats.
package/PartSegImage/image_reader.py
Outdated
| if ( | ||
| len(self.ranges) == 1 | ||
| and isinstance(self.colors, np.ndarray) | ||
| and self.colors.ndim == 2 | ||
| and self.colors.shape[0] > 1 | ||
| ): | ||
| return [ChannelInfo(name=self.channel_names[0], color_map=self.colors, contrast_limits=self.ranges[0])] |
There was a problem hiding this comment.
Add bounds check for channel_names before accessing index 0.
When self.ranges has exactly one entry and self.colors is a 2D array, the code accesses self.channel_names[0] without verifying that channel_names is non-empty, risking an IndexError if the channel name list is missing or empty.
Consider adding a defensive check or defaulting to a fallback name if channel_names is empty.
Apply this diff to add a safe default:
if (
len(self.ranges) == 1
and isinstance(self.colors, np.ndarray)
and self.colors.ndim == 2
and self.colors.shape[0] > 1
):
- return [ChannelInfo(name=self.channel_names[0], color_map=self.colors, contrast_limits=self.ranges[0])]
+ name = self.channel_names[0] if self.channel_names else "channel 1"
+ return [ChannelInfo(name=name, color_map=self.colors, contrast_limits=self.ranges[0])]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| len(self.ranges) == 1 | |
| and isinstance(self.colors, np.ndarray) | |
| and self.colors.ndim == 2 | |
| and self.colors.shape[0] > 1 | |
| ): | |
| return [ChannelInfo(name=self.channel_names[0], color_map=self.colors, contrast_limits=self.ranges[0])] | |
| if ( | |
| len(self.ranges) == 1 | |
| and isinstance(self.colors, np.ndarray) | |
| and self.colors.ndim == 2 | |
| and self.colors.shape[0] > 1 | |
| ): | |
| name = self.channel_names[0] if self.channel_names else "channel 1" | |
| return [ChannelInfo(name=name, color_map=self.colors, contrast_limits=self.ranges[0])] |
🤖 Prompt for AI Agents
In package/PartSegImage/image_reader.py around lines 138 to 144, the code
accesses self.channel_names[0] without ensuring channel_names is non-empty; add
a guard and fallback: extend the existing if-condition to require that
self.channel_names exists and has length > 0 (or else use a fallback name), and
when constructing ChannelInfo use either self.channel_names[0] or a safe default
like "Channel 0" (or an empty string) to avoid IndexError; keep the rest of the
logic unchanged.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
package/PartSegImage/image.py (1)
47-60: Convert existing numpy arrays to uint8 before validation.The current logic only converts non-ndarray inputs to uint8 (line 49). If a numpy array with
dtype=float64is passed directly, it raises aValueErrorat line 53 rather than being converted. The past review comments and pipeline failures confirm this causes issues.Apply this diff to convert existing numpy arrays:
def __post_init__(self): - if not isinstance(self.color_map, (str, np.ndarray)): - self.color_map = np.array(self.color_map, dtype=np.uint8) - if isinstance(self.color_map, np.ndarray): - if self.color_map.dtype != np.uint8: - message = f"Colormap as array need to be uint8, not {self.color_map.dtype}" - raise ValueError(message) + if not isinstance(self.color_map, str): + # Convert all non-string inputs to uint8 arrays + if isinstance(self.color_map, np.ndarray): + self.color_map = self.color_map.astype(np.uint8) + else: + self.color_map = np.array(self.color_map, dtype=np.uint8) + + # Validate shape if self.color_map.ndim in {1, 2}: if self.color_map.shape[0] not in {3, 4}:
🧹 Nitpick comments (1)
package/PartSegImage/image.py (1)
936-960: Consider explicit int conversion for robustness.At line 953,
tuple(color)produces a tuple of numpy scalars whencoloris a numpy array. While_rgb_to_signed_intusesnp.int32()casts that should handle numpy scalars, explicit conversion to Python ints would be more robust and avoid potential type confusion.Apply this diff for clarity:
elif color.ndim == 1: # treat as RGB - res.append(_rgb_to_signed_int(tuple(color))) + res.append(_rgb_to_signed_int(tuple(int(x) for x in color[:3])))Based on learnings
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package/PartSegImage/image.py(4 hunks)package/PartSegImage/image_reader.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T10:23:14.634Z
Learnt from: Czaki
PR: 4DNucleome/PartSeg#1312
File: package/PartSegImage/image.py:925-949
Timestamp: 2025-10-11T10:23:14.634Z
Learning: In package/PartSegImage/image.py, the ChannelInfoFull dataclass's __post_init__ method automatically converts any color_map that is not a string or numpy.ndarray to a numpy array, ensuring that self.default_coloring (which returns [x.color_map for x in self._channel_info]) only contains strings or numpy arrays, never raw tuples or lists.
Applied to files:
package/PartSegImage/image.py
🧬 Code graph analysis (1)
package/PartSegImage/image_reader.py (1)
package/PartSegImage/image.py (3)
dtype(458-460)shape(651-653)spacing(770-772)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: ubuntu-24.04 py3.12 --pre
- GitHub Check: macos-14 py3.12 --pre
- GitHub Check: windows-2022 py3.12 --pre
- GitHub Check: Base py3.11 / ubuntu-24.04 py 3.11 latest PyQt5
- GitHub Check: Base py3.12 / ubuntu-24.04 py 3.12 latest PyQt5
- GitHub Check: Base py3.10 / ubuntu-24.04 py 3.10 latest PyQt5
- GitHub Check: Base py3.12 / ubuntu-22.04 py 3.12 latest PyQt6
- GitHub Check: Base py3.10 / ubuntu-24.04 py 3.10 latest PySide6
- GitHub Check: Base py3.10 / ubuntu-22.04 py 3.10 latest PySide2
- GitHub Check: Base py3.11 / macos-15-intel py 3.11 latest PyQt5
- GitHub Check: test_coverage / ubuntu-24.04 py 3.12 latest PyQt5
- GitHub Check: Test PartSeg conda
- GitHub Check: repo py3.12
- GitHub Check: repo py3.10
- GitHub Check: repo py3.11
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
🔇 Additional comments (5)
package/PartSegImage/image_reader.py (3)
593-595: LGTM! Enhanced type annotation improves code clarity.The precise return type annotation for
_read_imagej_colorsclearly documents that it returns either a single 2D uint8 array or a list of such arrays, improving IDE support and maintainability.
598-598: LGTM! Typo correction.Fixed "PArtSeg" → "PartSeg" in the comment.
609-615: LGTM! Correct handling of single-channel color arrays.The logic correctly wraps 2D color arrays in a single-element list when there's no 'C' axis (single-channel case), ensuring consistent list structure for downstream processing.
package/PartSegImage/image.py (2)
911-934: LGTM! Indentation issue resolved.The
get_imagej_colorsmethod now correctly placesres.append(color.astype(np.uint8))in the else block (lines 932-933), preventing double appends that would occur in the previous version.
1042-1045: LGTM! Proper handling of RGB to signed integer conversion.The function correctly converts RGB tuples to signed 32-bit integers using
np.int32()casts before bit operations. This handles both Python ints and numpy scalars properly, with the final result encoded as RGBA with full alpha (255).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package/PartSegImage/image.py (1)
1045-1048: Use uint32 for bit operations to avoid signed overflow, and return Python int.The current implementation uses
np.int32for bit-shifting, which can cause signed integer overflow. When shiftingnp.int32(255) << 24, the result exceeds the int32 range (2,147,483,647), leading to unexpected wrapping behavior.Additionally, the return type annotation indicates
-> int(Python int), but the function returnsnp.int32(numpy scalar).Apply this diff to fix both issues:
def _rgb_to_signed_int(rgb: tuple[int, int, int]) -> int: """Convert an RGB tuple to a signed integer representation.""" - r, g, b = rgb[:3] - return np.int32((np.int32(r) << 24) | (np.int32(g) << 16) | (np.int32(b) << 8) | np.int32(255)) + r, g, b = (int(x) for x in rgb[:3]) + return int(np.uint32((r << 24) | (g << 16) | (b << 8) | 255).view(np.int32))This approach:
- Converts RGB components to Python ints to handle numpy scalars
- Uses
uint32for bit operations to avoid signed overflow- Uses
.view(np.int32)to reinterpret bits as signed int32- Wraps in
int()to return Python int as annotated
🧹 Nitpick comments (1)
package/PartSegImage/image.py (1)
49-63: Consider converting numpy array colormaps to uint8 instead of strict validation.The current implementation raises
ValueErrorfor numpy arrays with incorrect dtype (e.g., float64) or shape. While this provides clear error messages, it may be less user-friendly than automatically converting to the expected format.Consider adding automatic conversion for existing numpy arrays:
if not isinstance(self.color_map, str): if isinstance(self.color_map, np.ndarray): self.color_map = self.color_map.astype(np.uint8) else: self.color_map = np.array(self.color_map, dtype=np.uint8) # Validate shape after conversion if self.color_map.ndim in {1, 2}: if self.color_map.shape[0] not in {3, 4}: message = f"Color map need to have 3 or 4 elements (RGB or RGBA), not {self.color_map.shape}" raise ValueError(message) else: message = f"Colormap as sequence need to be 1d or 2d array, not {self.color_map.shape}" raise ValueError(message)This would handle cases where users provide
np.linspace((0, 0, 0), (128, 255, 0), num=256).Twithout explicitly specifyingdtype=np.uint8.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package/PartSegImage/image.py(4 hunks)package/PartSegImage/image_reader.py(1 hunks)package/tests/test_PartSegImage/test_image_writer.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T10:23:14.634Z
Learnt from: Czaki
PR: 4DNucleome/PartSeg#1312
File: package/PartSegImage/image.py:925-949
Timestamp: 2025-10-11T10:23:14.634Z
Learning: In package/PartSegImage/image.py, the ChannelInfoFull dataclass's __post_init__ method automatically converts any color_map that is not a string or numpy.ndarray to a numpy array, ensuring that self.default_coloring (which returns [x.color_map for x in self._channel_info]) only contains strings or numpy arrays, never raw tuples or lists.
Applied to files:
package/PartSegImage/image.py
🧬 Code graph analysis (2)
package/tests/test_PartSegImage/test_image_writer.py (3)
package/PartSegImage/image.py (6)
dtype(461-463)ChannelInfo(35-38)get_colors(965-974)channel_names(431-432)ranges(392-393)default_coloring(396-397)package/PartSegImage/image_writer.py (5)
IMAGEJImageWriter(116-175)save(16-17)save(65-81)save(120-140)ImageWriter(25-113)package/PartSegImage/image_reader.py (3)
TiffImageReader(460-677)read_image(167-188)read_image(267-288)
package/PartSegImage/image_reader.py (1)
package/PartSegImage/image.py (1)
spacing(773-775)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: ubuntu-24.04 py3.12 --pre
- GitHub Check: windows-2022 py3.12 --pre
- GitHub Check: repo py3.12
- GitHub Check: repo py3.11
- GitHub Check: repo py3.10
- GitHub Check: Base py3.11 / macos-15-intel py 3.11 latest PyQt5
- GitHub Check: test_coverage / ubuntu-24.04 py 3.12 latest PyQt5
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
🔇 Additional comments (7)
package/tests/test_PartSegImage/test_image_writer.py (3)
100-124: LGTM!The test fixture provides comprehensive coverage of different color map formats (named colors, hex codes, RGB tuples, and gradient arrays), which aligns well with the new OME color encoding functionality.
126-136: LGTM!The test correctly verifies that ImageJ TIFF format preserves all color definitions including gradient colormaps.
138-146: LGTM!The test correctly validates that OME format falls back to magenta for the gradient colormap in channel 5, as 2D gradient colormaps are not supported in OME color metadata (which stores colors as single integers). This aligns with the logging warning in
get_ome_colorsfor custom colormaps.package/PartSegImage/image.py (3)
3-3: LGTM!The logging import is appropriately added to support warning messages for unsupported custom colormaps in OME format.
914-937: LGTM!The enhanced handling for 2D colormaps correctly interpolates non-256-width colormaps to the standard 256 entries and ensures uint8 dtype conversion.
939-963: LGTM!The method correctly encodes per-channel colors as OME-compatible signed integers, with appropriate fallback to default colors for unsupported 2D gradient colormaps.
package/PartSegImage/image_reader.py (1)
593-610: LGTM!The type annotations accurately reflect the return type, and the function correctly wraps numpy arrays in a list to maintain a consistent
list[np.ndarray]return type.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
package/PartSegImage/image.py (1)
1050-1054: Correct the signed integer bit-shift operations and return type.The current implementation has several issues:
- Type mismatch: Returns
np.int32(numpy scalar) but signature saysint(Python int)- Unsafe bit operations: Bit-shifting on signed integers (
np.int32) can produce unexpected results due to sign bit handling- Missing conversion: No
.item()call to convert numpy scalar to Python intPer the past review discussion, bit operations should be performed on unsigned integers, then viewed as signed.
Apply this fix to use the safe unsigned-then-signed approach:
def _rgb_to_signed_int(rgb: tuple[int, int, int]) -> int: """Convert an RGB tuple to a signed integer representation.""" - r, g, b = rgb[:3] - return np.int32((np.int32(r) << 24) | (np.int32(g) << 16) | (np.int32(b) << 8) | np.int32(255)) + # Convert to Python ints to handle numpy scalars + r, g, b = int(rgb[0]), int(rgb[1]), int(rgb[2]) + # Perform bit operations on unsigned, then view as signed + return np.uint32((r << 24) | (g << 16) | (b << 8) | 255).view(np.int32).item()This ensures:
- Numpy scalars are converted to Python ints before operations
- Bit operations are performed on unsigned integers (avoiding sign bit issues)
- Result is correctly converted to a Python int matching the return type
Based on past review comments.
🧹 Nitpick comments (1)
package/PartSegImage/image.py (1)
54-60: Consider auto-converting arrays to uint8 for better UX.The current strict validation (raising errors for non-uint8 dtypes) is a valid design choice that enforces correct input. However, automatically converting arrays to uint8 before validation would provide a better user experience by accepting common formats like float arrays.
If you prefer auto-conversion, apply this diff:
if isinstance(self.color_map, np.ndarray): + # Auto-convert to uint8 for better UX + if self.color_map.dtype != np.uint8: + self.color_map = self.color_map.astype(np.uint8) - if self.color_map.dtype != np.uint8: - message = f"Colormap as array need to be uint8, not {self.color_map.dtype}" - raise ValueError(message) if self.color_map.ndim in {1, 2}:
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package/PartSegImage/image.py(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T10:23:14.634Z
Learnt from: Czaki
PR: 4DNucleome/PartSeg#1312
File: package/PartSegImage/image.py:925-949
Timestamp: 2025-10-11T10:23:14.634Z
Learning: In package/PartSegImage/image.py, the ChannelInfoFull dataclass's __post_init__ method automatically converts any color_map that is not a string or numpy.ndarray to a numpy array, ensuring that self.default_coloring (which returns [x.color_map for x in self._channel_info]) only contains strings or numpy arrays, never raw tuples or lists.
Applied to files:
package/PartSegImage/image.py
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: repo py3.11
- GitHub Check: repo py3.10
- GitHub Check: repo py3.12
- GitHub Check: Base py3.11 / macos-15-intel py 3.11 latest PyQt5
- GitHub Check: test_coverage / ubuntu-24.04 py 3.12 latest PyQt5
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
🔇 Additional comments (3)
package/PartSegImage/image.py (3)
43-44: Good addition of documentation.The docstring clarifies the purpose of this dataclass within the Image class.
914-942: LGTM! Well-structured color conversion with detailed type annotation.The explicit return type annotation enhances type safety, and the implementation correctly handles all color format cases (string hex, named colors, 1D and 2D arrays).
944-968: Logic is sound, assuming _rgb_to_signed_int handles numpy scalars correctly.The method correctly handles all color map types with appropriate fallbacks. The validation in
__post_init__ensures 1D arrays always have 3-4 elements, so the length concern from past reviews is adequately addressed.Based on learnings.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
package/PartSegImage/image.py(5 hunks)package/PartSegImage/image_reader.py(1 hunks)package/PartSegImage/image_writer.py(1 hunks)package/tests/test_PartSegImage/test_image.py(2 hunks)package/tests/test_PartSegImage/test_image_writer.py(2 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Czaki
PR: 4DNucleome/PartSeg#1312
File: package/PartSegImage/image.py:925-949
Timestamp: 2025-10-11T10:23:14.634Z
Learning: In package/PartSegImage/image.py, the ChannelInfoFull dataclass's __post_init__ method automatically converts any color_map that is not a string or numpy.ndarray to a numpy array, ensuring that self.default_coloring (which returns [x.color_map for x in self._channel_info]) only contains strings or numpy arrays, never raw tuples or lists.
📚 Learning: 2025-10-11T10:23:14.634Z
Learnt from: Czaki
PR: 4DNucleome/PartSeg#1312
File: package/PartSegImage/image.py:925-949
Timestamp: 2025-10-11T10:23:14.634Z
Learning: In package/PartSegImage/image.py, the ChannelInfoFull dataclass's __post_init__ method automatically converts any color_map that is not a string or numpy.ndarray to a numpy array, ensuring that self.default_coloring (which returns [x.color_map for x in self._channel_info]) only contains strings or numpy arrays, never raw tuples or lists.
Applied to files:
package/PartSegImage/image.py
🪛 Ruff (0.14.0)
package/tests/test_PartSegImage/test_image.py
736-736: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
🔇 Additional comments (13)
package/tests/test_PartSegImage/test_image_writer.py (3)
100-123: LGTM! Comprehensive fixture for multi-channel color testing.The fixture creates a 5-channel image with diverse color definitions (string name, hex codes, RGB tuple, and 2D colormap), providing excellent coverage for the new color handling functionality.
126-136: LGTM! ImageJ color persistence validated.The test correctly verifies that all 5 channel colors (including the gradient colormap) are properly saved and restored when using ImageJ TIFF format.
138-146: LGTM! OME color encoding validated.The test correctly verifies that OME-TIFF saves colors as single RGB tuples, with appropriate fallback to magenta for the custom gradient colormap (channel 5) which is not supported in OME color format.
package/PartSegImage/image_writer.py (1)
76-80: LGTM! OME color metadata properly integrated.The addition of the
Colorfield usingimage.get_ome_colors()correctly extends the OME-TIFF Channel metadata with per-channel color information.package/tests/test_PartSegImage/test_image.py (2)
5-5: LGTM! Necessary import for array assertions.The
numpy.testingimport asnptis correctly added to support precise array comparisons in the new tests.
734-771: LGTM! Comprehensive validation tests.The test class provides thorough coverage of
ChannelInfoFullinitialization with:
- Various input types (tuple, list, numpy array) for both 1D and 2D colormaps
- Proper validation of shape, dtype, and dimensionality constraints
- Clear error cases with expected messages
The static analysis warning about
RUF012can be ignored—these are test constants, not mutable class attributes.package/PartSegImage/image_reader.py (2)
593-610: LGTM! Type annotations enhance code clarity.The addition of precise type annotations for
_read_imagej_colors(parameter typetifffile.TiffFileand return typelist[np.ndarray[...]]) improves type safety and IDE support. The expanded docstring also helps document the method's behavior.Note: Past review comments indicate that unreachable code issues have been addressed.
612-632: LGTM! Consistent type annotation improvements.The type annotations for
read_imagej_metadata(parameter typetifffile.TiffFile, return typeNone) are consistent with the rest of the changes and improve code documentation. The expanded docstring clearly describes what metadata is read and where it's stored.package/PartSegImage/image.py (4)
3-3: LGTM! Logging import supports warnings.The logging import is correctly added to support warning messages in
get_ome_colorsabout unsupported colormap types.
49-63: LGTM! Color validation properly enforces constraints.The
__post_init__validation correctly:
- Converts non-string, non-ndarray inputs to
uint8numpy arrays- Validates that arrays have
uint8dtype- Enforces shape constraints (1D or 2D with 3 or 4 RGB/RGBA elements)
- Provides clear error messages for validation failures
Based on past review comments, the conversion to
uint8before validation was correctly implemented.Based on learnings.
914-942: LGTM! ImageJ color conversion properly typed and implemented.The method now has precise type annotations and correctly handles all color types:
- String colors (hex codes and named colors) → interpolated to 3×256
- 1D RGB arrays → interpolated to 3×256
- 2D colormaps → interpolated to 3×256 if needed, or cast to uint8
Based on past review comments, the indentation issue that caused double appending has been fixed.
1050-1053: LGTM! RGB-to-signed-int conversion correctly handles numpy types.The function properly converts RGB tuples to signed 32-bit integers using bit-shifting. Based on past review comments, the code now correctly casts values to
np.int32before bit operations to handle numpy scalar types (e.g.,np.uint8), preventingTypeErrorduring bit-shifting operations.Based on past review comments.
pyproject.toml (1)
3-3: Clarify necessity of setuptools>=77.0.0setuptools 77.0.0 is available on PyPI (current latest is 80.9.0). Please confirm which feature or bug-fix in v77 is required for this PR, or document the bump rationale in the commit message.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
package/PartSegImage/image.py (1)
49-63: Convert numpy arrays to uint8 before dtype validation.The current logic only converts non-ndarray inputs to
uint8at line 52. If a numpy array withdtype=float64is passed directly, it fails at line 54's dtype check rather than being converted. The pipeline failures confirm this issue.Based on past review comments.
Apply this diff to convert all arrays to uint8 before validation:
def __post_init__(self): """Normalize color_map to numpy array if it is not string.""" - if not isinstance(self.color_map, (str, np.ndarray)): + if not isinstance(self.color_map, str): + if isinstance(self.color_map, np.ndarray): + self.color_map = self.color_map.astype(np.uint8) + else: + self.color_map = np.array(self.color_map, dtype=np.uint8) + + if isinstance(self.color_map, np.ndarray): - self.color_map = np.array(self.color_map, dtype=np.uint8) - if isinstance(self.color_map, np.ndarray): - if self.color_map.dtype != np.uint8: - message = f"Colormap as array need to be uint8, not {self.color_map.dtype}" - raise ValueError(message) if self.color_map.ndim in {1, 2}: if self.color_map.shape[0] not in {3, 4}: message = f"Color map need to have 3 or 4 elements (RGB or RGBA), not {self.color_map.shape}" raise ValueError(message) else: message = f"Colormap as sequence need to be 1d or 2d array, not {self.color_map.shape}" raise ValueError(message)
🧹 Nitpick comments (1)
package/PartSegImage/image.py (1)
1050-1054: Consider simplifying the implementation.The function correctly handles numpy scalars by wrapping in
np.int32(). The logic is sound.Consider this simpler implementation:
def _rgb_to_signed_int(rgb: tuple[int, int, int]) -> int: """Convert an RGB tuple to a signed integer representation.""" - r, g, b = rgb[:3] - return np.int32((np.int32(r) << 24) | (np.int32(g) << 16) | (np.int32(b) << 8) | np.int32(255)) + r, g, b = (int(x) for x in rgb[:3]) + return np.int32((r << 24) | (g << 16) | (b << 8) | 255)This converts to Python integers first, avoiding the need for multiple
np.int32()calls.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package/PartSegImage/image.py(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T10:23:14.634Z
Learnt from: Czaki
PR: 4DNucleome/PartSeg#1312
File: package/PartSegImage/image.py:925-949
Timestamp: 2025-10-11T10:23:14.634Z
Learning: In package/PartSegImage/image.py, the ChannelInfoFull dataclass's __post_init__ method automatically converts any color_map that is not a string or numpy.ndarray to a numpy array, ensuring that self.default_coloring (which returns [x.color_map for x in self._channel_info]) only contains strings or numpy arrays, never raw tuples or lists.
Applied to files:
package/PartSegImage/image.py
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)
- GitHub Check: Base py3.11 / macos-15-intel py 3.11 latest PyQt5
- GitHub Check: Base py3.12 / ubuntu-22.04 py 3.12 latest PyQt6
- GitHub Check: Base py3.10 / ubuntu-24.04 py 3.10 latest PySide6
- GitHub Check: Base py3.12 / ubuntu-24.04 py 3.12 latest PyQt5
- GitHub Check: Base py3.11 / ubuntu-24.04 py 3.11 latest PyQt5
- GitHub Check: Base py3.10 / ubuntu-24.04 py 3.10 latest PyQt5
- GitHub Check: Base py3.12 / macos-15 py 3.12 latest PyQt6
- GitHub Check: Base py3.11 / windows-latest py 3.11 latest PyQt5
- GitHub Check: Base py3.10 / ubuntu-22.04 py 3.10 latest PySide2
- GitHub Check: Base py3.9 / ubuntu-24.04 py 3.9 latest PyQt5
- GitHub Check: Test PartSeg minimal / ubuntu-24.04 py 3.9 latest PyQt5
- GitHub Check: test_coverage / ubuntu-24.04 py 3.12 latest PyQt5
- GitHub Check: Test PartSeg conda
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: Sourcery review
🔇 Additional comments (3)
package/PartSegImage/image.py (3)
3-3: LGTM!The logging import is correctly added to support warning messages in the new
get_ome_colorsmethod.
914-942: LGTM!The method correctly handles all color format variations with proper type annotations and clear logic flow. The if-elif-else chain ensures each color is appended exactly once.
944-968: LGTM! Verify validation prevents short arrays.The method correctly handles string colors, RGB arrays, and RGBA arrays (using
[:3]slice at line 961). The fallback to default colors for 2D colormaps is appropriate with logging.Ensure the validation fix in
__post_init__is applied to prevent 1D arrays with fewer than 3 elements from reaching this method.Based on learnings.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package/PartSegImage/image.py (1)
1050-1054: Return Python int for type consistency.The return type annotation specifies
int(Python integer), but the function returnsnp.int32. While numpy scalars are generally compatible, explicitly converting to Python int improves type consistency and avoids potential issues with type checkers.Apply this diff:
def _rgb_to_signed_int(rgb: tuple[int, int, int]) -> int: """Convert an RGB tuple to a signed integer representation.""" r, g, b = (np.int32(x) for x in rgb[:3]) - return np.int32((r << 24) | (g << 16) | (b << 8) | np.int32(255)) + return int((r << 24) | (g << 16) | (b << 8) | 255)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package/PartSegImage/image.py(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T10:23:14.634Z
Learnt from: Czaki
PR: 4DNucleome/PartSeg#1312
File: package/PartSegImage/image.py:925-949
Timestamp: 2025-10-11T10:23:14.634Z
Learning: In package/PartSegImage/image.py, the ChannelInfoFull dataclass's __post_init__ method automatically converts any color_map that is not a string or numpy.ndarray to a numpy array, ensuring that self.default_coloring (which returns [x.color_map for x in self._channel_info]) only contains strings or numpy arrays, never raw tuples or lists.
Applied to files:
package/PartSegImage/image.py
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Tests_linux test_linux)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: Base py3.11 / macos-15-intel py 3.11 latest PyQt5
- GitHub Check: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
🔇 Additional comments (3)
package/PartSegImage/image.py (3)
3-3: LGTM!The logging import is necessary for the warning in
get_ome_colorsand correctly placed with other standard library imports.
914-942: LGTM!The updated type annotation provides clear documentation of the return format (list of 3×256 uint8 arrays), and the logic correctly handles all color map formats: named colors, hex codes, 1D RGB arrays, and 2D colormaps with proper resampling.
944-968: LGTM!The method correctly encodes channel colors for OME-TIFF format:
- Handles string colors (hex and named) with proper conversion to RGB
- Extracts RGB from 1D arrays using
[:3], correctly handling both RGB and RGBA inputs- Falls back gracefully with a warning for unsupported custom colormaps
Based on learnings.
1 participant
Summary by Sourcery
Add support for saving channel colormaps in OME-TIFF outputs by encoding colors as signed integers and embedding them in the Channel metadata.
New Features:
Enhancements:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests