chore: Scripts for segmentation dilation and max projection #1307
Conversation
Reviewer's GuideIntroduce two example CLI scripts: one for dilating segmentation masks by leveraging PartSegCore’s ROI operations and Euclidean sprawl computation, and another for generating max-intensity projections of TIFF stacks using PartSegImage utilities. Sequence diagram for dilate_segmentation_mask.py CLI workflowsequenceDiagram
actor User
participant Script
participant "PartSegCore"
participant "PartSegCore_compiled_backend"
User->>Script: Run dilate_segmentation_mask.py with files, radius, suffix
Script->>"PartSegCore": Load segmentation mask file
Script->>"PartSegCore": Convert mask to binary
Script->>"PartSegCore": Dilate binary mask
Script->>"PartSegCore": Calculate distances and neighbors
Script->>"PartSegCore_compiled_backend": Compute euclidean sprawl
Script->>"PartSegCore": Fit array to image
Script->>"PartSegCore": Save new mask file
Sequence diagram for max_projection.py CLI workflowsequenceDiagram
actor User
participant Script
participant "PartSegImage"
User->>Script: Run max_projection.py with image files and suffix
Script->>"PartSegImage": Read TIFF image
Script->>"PartSegImage": Compute max projection along Z axis
Script->>"PartSegImage": Create new image object
Script->>"PartSegImage": Save new image file
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds three new example scripts: Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as dilate_segmentation_mask.py
participant FS as FileSystem
participant ROIops as ROI/Image Ops
participant Saver as SaveROI
User->>CLI: Run with file patterns, --radius, --suffix, [--only-selected]
CLI->>CLI: Parse args, expand globs
loop For each file
CLI->>FS: Load ROI file
CLI->>ROIops: Optionally filter to selected components (remap values)
CLI->>ROIops: Binarize, compute dilation & neighborhood distances
ROIops->>ROIops: Apply euclidean_sprawl and refit to image metadata
ROIops->>Saver: Prepare updated ROIInfo/metadata and new filename
Saver->>FS: Write output ROI file with suffix
end
sequenceDiagram
autonumber
actor User
participant CLI as max_projection.py
participant FS as FileSystem
participant TIFF as TiffImageReader
participant Proc as Projection
User->>CLI: Run with file patterns, --suffix, [--with-mask]
CLI->>CLI: Parse args, expand globs
loop For each file
CLI->>TIFF: Read TIFF stack
TIFF-->>CLI: Return image with Z axis
CLI->>Proc: Compute max along Z (and optional mask projection)
Proc-->>CLI: Return reduced-dimension Image(s) with updated axes/spacings
CLI->>FS: Save output(s) with suffix(es)
end
sequenceDiagram
autonumber
actor User
participant CLI as extract_components_from_project.py
participant FS as FileSystem
participant Loader as LoadROIImage
participant Saver as SaveComponents
User->>CLI: Run with project file globs
CLI->>CLI: Parse args, expand globs
loop For each file
CLI->>Loader: Load ROI image
Loader-->>CLI: Return ROI image data
CLI->>Saver: Call SaveComponents.save(...) to export components
Saver->>FS: Write `_components` output file
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (25)
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 and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `examples/max_projection.py:15` </location>
<code_context>
+
+def max_projection(file_path: Path, suffix: str = "_max"):
+ image = TiffImageReader.read_image(str(file_path))
+ max_proj = image.get_data().max(axis=image.axis_order.index("Z"))
+ image2 = Image(max_proj, image.spacing[1:], axes_order=image.axis_order.replace("Z", ""))
+ ImageWriter.save(image2, str(file_path.with_name(file_path.stem + suffix + file_path.suffix)))
</code_context>
<issue_to_address>
**issue:** Potential issue if 'Z' axis is missing from axis_order.
Add a check to ensure 'Z' is present in axis_order before calling index(), or handle cases where 'Z' is missing to prevent ValueError.
</issue_to_address>
### Comment 2
<location> `examples/max_projection.py:16` </location>
<code_context>
+def max_projection(file_path: Path, suffix: str = "_max"):
+ image = TiffImageReader.read_image(str(file_path))
+ max_proj = image.get_data().max(axis=image.axis_order.index("Z"))
+ image2 = Image(max_proj, image.spacing[1:], axes_order=image.axis_order.replace("Z", ""))
+ ImageWriter.save(image2, str(file_path.with_name(file_path.stem + suffix + file_path.suffix)))
+
</code_context>
<issue_to_address>
**issue:** Assumes spacing[1:] matches axes after removing 'Z'.
Explicitly map spacing to axes to ensure correct correspondence, especially if axis order varies.
</issue_to_address>
### Comment 3
<location> `examples/dilate_segmentation_mask.py:19` </location>
<code_context>
+
+def convert_mask(file_path: Path, radius: float, suffix: str):
+ print(f"Converting {file_path} to {suffix} with radius {radius}")
+ if radius <= 0:
+ return
+ project = LoadROIImage.load([str(file_path)])
</code_context>
<issue_to_address>
**suggestion:** Early return for non-positive radius may silently skip files.
Consider adding a warning or error log when skipping files due to non-positive radius to inform the user.
Suggested implementation:
```python
import logging
def convert_mask(file_path: Path, radius: float, suffix: str):
print(f"Converting {file_path} to {suffix} with radius {radius}")
if radius <= 0:
logging.warning(f"Skipping {file_path}: radius must be positive, got {radius}")
return
project = LoadROIImage.load([str(file_path)])
```
If your codebase does not already use the `logging` module, you may want to configure it at the top of your script, e.g.:
```python
import logging
logging.basicConfig(level=logging.INFO)
```
Alternatively, if you prefer not to use logging, replace `logging.warning(...)` with `print(f"WARNING: Skipping {file_path}: radius must be positive, got {radius}")`.
</issue_to_address>
### Comment 4
<location> `examples/dilate_segmentation_mask.py:66-67` </location>
<code_context>
+
+ args = parser.parse_args()
+
+ files = list(chain.from_iterable(glob(x) for x in args.project_files))
+ print(f"{files=} {args.project_files=}")
+
</code_context>
<issue_to_address>
**suggestion:** No check for empty file list after glob expansion.
Add a warning or error if no files are found to improve user feedback.
```suggestion
files = list(chain.from_iterable(glob(x) for x in args.project_files))
print(f"{files=} {args.project_files=}")
if not files:
print("Warning: No files found matching the provided patterns.", flush=True)
exit(1)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
examples/max_projection.py
Outdated
| def max_projection(file_path: Path, suffix: str = "_max"): | ||
| image = TiffImageReader.read_image(str(file_path)) | ||
| max_proj = image.get_data().max(axis=image.axis_order.index("Z")) | ||
| image2 = Image(max_proj, image.spacing[1:], axes_order=image.axis_order.replace("Z", "")) |
There was a problem hiding this comment.
issue: Assumes spacing[1:] matches axes after removing 'Z'.
Explicitly map spacing to axes to ensure correct correspondence, especially if axis order varies.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
examples/max_projection.py (1)
20-28: Add error handling for individual file processing.The script will terminate on the first error without processing remaining files. Consider wrapping the processing in a try-except block to handle errors gracefully.
def main(): parser = ArgumentParser() parser.add_argument("image_files", nargs="+", type=str) parser.add_argument("--suffix", type=str, default="_max") args = parser.parse_args() files = list(chain.from_iterable(glob(f) for f in args.image_files)) + if not files: + print("Warning: No files matched the provided patterns") + return for file_path in files: print(f"Processing {file_path}") - max_projection(Path(file_path), args.suffix) + try: + max_projection(Path(file_path), args.suffix) + print(f" ✓ Successfully saved max projection") + except Exception as e: + print(f" ✗ Failed: {e}")examples/dilate_segmentation_mask.py (2)
18-20: Move print statement after validation.The progress message is printed before checking if
radius <= 0, leading to misleading output when no processing occurs.- print(f"Converting {file_path} to {suffix} with radius {radius}") if radius <= 0: + print(f"Skipping {file_path}: radius must be positive") return + print(f"Converting {file_path} to {suffix} with radius {radius}")
69-70: Consider adding error handling for robustness.Like in
max_projection.py, wrapping individual file processing in try-except would prevent the script from terminating on the first error.for file_path in files: - convert_mask(Path(file_path).absolute(), args.dilate, args.suffix) + try: + convert_mask(Path(file_path).absolute(), args.dilate, args.suffix) + except Exception as e: + print(f"Failed to process {file_path}: {e}")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/dilate_segmentation_mask.py(1 hunks)examples/max_projection.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
examples/dilate_segmentation_mask.py (5)
package/PartSegCore/image_operations.py (2)
dilate(96-105)to_binary_image(138-140)package/PartSegCore/mask/io_functions.py (4)
LoadROIImage(410-452)MaskProjectTuple(62-111)SaveROI(529-554)SaveROIOptions(114-125)package/PartSegCore/roi_info.py (1)
ROIInfo(33-124)package/PartSegCore/segmentation/watershed.py (2)
calculate_distances_array(281-297)get_neigh(268-269)package/PartSegImage/image.py (2)
spacing(758-760)fit_array_to_image(555-565)
examples/max_projection.py (3)
package/PartSegImage/image.py (2)
Image(183-971)spacing(758-760)package/PartSegImage/image_writer.py (1)
ImageWriter(25-112)package/PartSegImage/image_reader.py (1)
TiffImageReader(460-653)
⏰ 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_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: Base py3.11 / macos-13 py 3.11 latest PyQt5
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- 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 (manifest_check manifest_check)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
🔇 Additional comments (6)
examples/max_projection.py (2)
1-10: LGTM! Clean imports.All necessary dependencies are imported. The module docstring clearly describes the script's purpose.
31-32: LGTM! Standard entry point.examples/dilate_segmentation_mask.py (4)
1-14: LGTM! Comprehensive imports.All necessary dependencies are imported for ROI mask dilation operations.
29-55: LGTM! Correct euclidean sprawl and save logic.The ROI dilation workflow is correctly implemented:
- Euclidean sprawl extends components within the dilated area
- Result is fitted to the image shape
- Metadata (spacing, frame_thickness) is properly preserved
- New ROIInfo is correctly constructed
73-74: LGTM! Standard entry point.
23-28: Clarify ROI dimensionality and matching dilation radius
roi_ = project.roi_info.roi.squeeze(): collapsing singleton dimensions can unpredictably change your mask shape (e.g.(1,Z,Y,X)→(Z,Y,X)or(Z,1,Y,X)→(Z,Y,X)). Ensure the squeezed shape aligns with whether you intend per‐slice or full‐volume operations.- Passing
[radius, radius]withlayer=Trueinvokes 2D dilation on each slice. For 3D sprawl, supply a radius list of length 3 (e.g.[radius]*3) and calldilate(..., layer=False).- To avoid hard-coding, derive the radius list from
roi_.ndim(orlen(project.image.spacing)) so it automatically matches your data’s dimensionality.
examples/max_projection.py
Outdated
| def max_projection(file_path: Path, suffix: str = "_max"): | ||
| image = TiffImageReader.read_image(str(file_path)) | ||
| max_proj = image.get_data().max(axis=image.axis_order.index("Z")) | ||
| image2 = Image(max_proj, image.spacing[1:], axes_order=image.axis_order.replace("Z", "")) | ||
| ImageWriter.save(image2, str(file_path.with_name(file_path.stem + suffix + file_path.suffix))) |
There was a problem hiding this comment.
Add Z-axis validation and fix spacing calculation.
The function has critical assumptions that may fail at runtime:
- Line 15:
image.axis_order.index("Z")will raiseValueErrorif the image doesn't contain a Z axis. - Line 16:
image.spacing[1:]assumes Z is the first dimension in spacing, butaxis_order.index("Z")could return any position. The spacing slice should match the actual Z position in the axis order.
Consider this safer approach:
def max_projection(file_path: Path, suffix: str = "_max"):
image = TiffImageReader.read_image(str(file_path))
+ if "Z" not in image.axis_order:
+ raise ValueError(f"Image {file_path} does not contain a Z axis")
+ z_index = image.axis_order.index("Z")
- max_proj = image.get_data().max(axis=image.axis_order.index("Z"))
- image2 = Image(max_proj, image.spacing[1:], axes_order=image.axis_order.replace("Z", ""))
+ max_proj = image.get_data().max(axis=z_index)
+ new_spacing = tuple(s for i, s in enumerate(image.spacing) if i != z_index)
+ image2 = Image(max_proj, new_spacing, axes_order=image.axis_order.replace("Z", ""))
ImageWriter.save(image2, str(file_path.with_name(file_path.stem + suffix + file_path.suffix)))📝 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.
| def max_projection(file_path: Path, suffix: str = "_max"): | |
| image = TiffImageReader.read_image(str(file_path)) | |
| max_proj = image.get_data().max(axis=image.axis_order.index("Z")) | |
| image2 = Image(max_proj, image.spacing[1:], axes_order=image.axis_order.replace("Z", "")) | |
| ImageWriter.save(image2, str(file_path.with_name(file_path.stem + suffix + file_path.suffix))) | |
| def max_projection(file_path: Path, suffix: str = "_max"): | |
| image = TiffImageReader.read_image(str(file_path)) | |
| if "Z" not in image.axis_order: | |
| raise ValueError(f"Image {file_path} does not contain a Z axis") | |
| z_index = image.axis_order.index("Z") | |
| max_proj = image.get_data().max(axis=z_index) | |
| new_spacing = tuple(s for i, s in enumerate(image.spacing) if i != z_index) | |
| image2 = Image(max_proj, new_spacing, axes_order=image.axis_order.replace("Z", "")) | |
| ImageWriter.save(image2, str(file_path.with_name(file_path.stem + suffix + file_path.suffix))) |
🤖 Prompt for AI Agents
In examples/max_projection.py around lines 13 to 17, the code assumes a Z axis
and slices spacing incorrectly; first check that "Z" is present in
image.axis_order and raise or return a clear error if not, compute z_idx =
image.axis_order.index("Z"), use that z_idx when taking the max (axis=z_idx) and
build the new spacing by removing the element at z_idx (e.g., new_spacing =
tuple(s for i,s in enumerate(image.spacing) if i != z_idx)), set new_axes =
image.axis_order.replace("Z", "") and construct the Image with
spacing=new_spacing and axes_order=new_axes before saving.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
examples/max_projection.py (1)
17-18: Fix spacing calculation to match Z-axis position.Line 18 assumes Z is the first dimension when slicing
image.spacing[1:], but Z can appear at any position inaxis_order. When Z is not first, the wrong spacing element is removed.Apply this diff to correctly remove the spacing dimension corresponding to Z:
- max_proj = image.get_data().max(axis=image.axis_order.index("Z")) - image2 = Image(max_proj, image.spacing[1:], axes_order=image.axis_order.replace("Z", "")) + z_index = image.axis_order.index("Z") + max_proj = image.get_data().max(axis=z_index) + new_spacing = tuple(s for i, s in enumerate(image.spacing) if i != z_index) + image2 = Image(max_proj, spacing=new_spacing, axes_order=image.axis_order.replace("Z", ""))
🧹 Nitpick comments (1)
examples/max_projection.py (1)
27-30: Consider validating the file list is non-empty.If glob patterns match no files, the script silently succeeds with no feedback. Adding a check improves user experience.
Apply this diff:
files = list(chain.from_iterable(glob(f) for f in args.image_files)) + if not files: + print("Warning: No files found matching the provided patterns") + return for file_path in files: print(f"Processing {file_path}") max_projection(Path(file_path), args.suffix)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/dilate_segmentation_mask.py(1 hunks)examples/max_projection.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
examples/dilate_segmentation_mask.py (5)
package/PartSegCore/image_operations.py (2)
dilate(96-105)to_binary_image(138-140)package/PartSegCore/mask/io_functions.py (4)
LoadROIImage(410-452)MaskProjectTuple(62-111)SaveROI(529-554)SaveROIOptions(114-125)package/PartSegCore/roi_info.py (1)
ROIInfo(33-124)package/PartSegCore/segmentation/watershed.py (2)
calculate_distances_array(281-297)get_neigh(268-269)package/PartSegImage/image.py (2)
spacing(758-760)fit_array_to_image(555-565)
examples/max_projection.py (3)
package/PartSegImage/image.py (2)
Image(183-971)spacing(758-760)package/PartSegImage/image_writer.py (1)
ImageWriter(25-112)package/PartSegImage/image_reader.py (1)
TiffImageReader(460-653)
🪛 Ruff (0.13.2)
examples/dilate_segmentation_mask.py
20-20: Avoid specifying long messages outside the exception class
(TRY003)
examples/max_projection.py
16-16: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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: Codacy Static Code Analysis
- 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: Base py3.11 / macos-13 py 3.11 latest PyQt5
- 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 (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: Sourcery review
🔇 Additional comments (8)
examples/max_projection.py (2)
5-10: LGTM!Imports are clean and all are utilized in the script.
33-34: LGTM!Standard entry point pattern is correctly implemented.
examples/dilate_segmentation_mask.py (6)
3-15: LGTM!All imports are necessary and correctly used throughout the script.
18-21: LGTM!Radius validation now correctly raises an error for non-positive values, and the
--dilateargument type has been fixed tofloatin the main function. The exception message is clear and appropriate for a CLI tool.Note: Static analysis flags the exception message as potentially too long (TRY003), but for an example script this is acceptable and improves usability.
23-39: LGTM!The dilation logic correctly:
- Loads the ROI project
- Converts to binary and dilates the mask
- Applies euclidean sprawl to preserve component boundaries
- Fits the result back to the image dimensions
The use of
squeeze()andlayer=Truewith 2D radius[radius, radius]is appropriate for layer-wise processing.
40-57: LGTM!The save logic correctly:
- Constructs the output filename with the specified suffix
- Creates a new MaskProjectTuple preserving original image and metadata
- Updates only the ROI with the dilated result
- Uses appropriate SaveROIOptions to maintain data integrity
60-75: LGTM!The main function correctly addresses all previous review concerns:
--dilateargument is nowtype=floatmatching the function signature- Empty file list validation provides user feedback
- Debug print statement has been removed
- Return codes indicate success (0) or failure (-1)
78-79: LGTM!Entry point correctly propagates the return code via
sys.exit(main()), enabling proper error detection in shell scripts.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1307 +/- ##
========================================
Coverage 93.16% 93.16%
========================================
Files 210 210
Lines 33232 33232
========================================
Hits 30959 30959
Misses 2273 2273 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
examples/dilate_segmentation_mask.py (1)
76-76: Type mismatch: use float instead of int for --dilate argument.The
--dilateargument is declared astype=int, butconvert_maskexpectsradius: float. While Python's duck typing allows integers to work as floats, this creates confusion and prevents users from specifying fractional radius values.Note: This issue was flagged in previous review comments and supposedly addressed in commits 0d7b527 to 3310031, but the current code still shows
type=int.Apply this diff to fix the type:
- parser.add_argument("--dilate", type=int, default=1) + parser.add_argument("--dilate", type=float, default=1.0)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/dilate_segmentation_mask.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/dilate_segmentation_mask.py (5)
package/PartSegCore/image_operations.py (2)
dilate(96-105)to_binary_image(138-140)package/PartSegCore/mask/io_functions.py (4)
LoadROIImage(410-452)MaskProjectTuple(62-111)SaveROI(529-554)SaveROIOptions(114-125)package/PartSegCore/roi_info.py (1)
ROIInfo(33-124)package/PartSegCore/segmentation/watershed.py (2)
calculate_distances_array(281-297)get_neigh(268-269)package/PartSegImage/image.py (3)
dtype(461-463)spacing(773-775)fit_array_to_image(570-580)
🪛 Ruff (0.14.0)
examples/dilate_segmentation_mask.py
20-20: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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: Prepare coverage
- 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 sdist)
- 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 (manifest_check manifest_check)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: Sourcery review
🔇 Additional comments (6)
examples/dilate_segmentation_mask.py (6)
1-16: LGTM! Clean import structure.The imports are well-organized and include all necessary modules for ROI dilation operations.
18-21: LGTM! Proper parameter validation.The function signature and radius validation are appropriate for this CLI example script.
23-30: LGTM! Correct ROI loading and filtering.The ROI loading and optional filtering logic correctly handles the selected components case.
52-70: LGTM! Proper file saving logic.The file path construction and SaveROI usage are correct. The new MaskProjectTuple properly includes the dilated ROI and preserves relevant metadata.
80-89: LGTM! Proper CLI argument handling.The file glob expansion, validation, and processing loop are correctly implemented. Good that the no-files-found check was added based on previous feedback.
92-93: LGTM! Standard main guard.The main guard and exit handling are properly implemented.
| selected_components = list(range(1, len(unique_values))) | ||
|
|
||
| bin_roi = to_binary_image(roi_) | ||
| sprawl_area = dilate(bin_roi, [radius, radius], True) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify 2D-only dilation is intentional.
The dilation uses [radius, radius] which appears to dilate only in X and Y dimensions, not Z. For 3D volumetric images, this might not produce the expected result. If Z-axis dilation is needed, consider using radius directly or [radius, radius, radius].
Run the following script to check if this project typically uses 3D images:
🏁 Script executed:
#!/bin/bash
# Check for 3D image handling in the codebase and verify dilation usage patterns
rg -nP -C3 'dilate\(' --type=py | head -50Length of output: 3798
🏁 Script executed:
#!/bin/bash
# Inspect dilate implementation
sed -n '90,150p' package/PartSegCore/image_operations.py
# Inspect example usage and components_num context
sed -n '1,200p' examples/dilate_segmentation_mask.pyLength of output: 4620
Ensure full 3D dilation and correct component count
- Replace 2D‐only dilation with full 3D, e.g.:
- sprawl_area = dilate(bin_roi, [radius, radius], True)
- sprawl_area = dilate(bin_roi, [radius, radius, radius], layer=False)
- Compute component count from the filtered ROI, not the original, e.g.:
- components_num = np.max(project.roi_info.roi)
- components_num = np.max(roi_)
📝 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.
| sprawl_area = dilate(bin_roi, [radius, radius], True) | |
| # … earlier in the function … | |
| sprawl_area = dilate(bin_roi, [radius, radius, radius], layer=False) | |
| components_num = np.max(roi_) | |
| # … later in the function … |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
examples/dilate_segmentation_mask.py (2)
18-21: LGTM! Input validation is correct.The radius validation properly raises
ValueErrorfor non-positive values. The error message is clear and appropriate for an example script.Optional refinement: The static analysis tool suggests defining a custom exception class to avoid long messages in
raisestatements (TRY003). For a simple example script, the current approach is fine, but for production code, consider:class InvalidRadiusError(ValueError): """Raised when radius is not positive.""" pass def convert_mask(file_path: Path, radius: float, suffix: str, only_selected: bool): if radius <= 0: raise InvalidRadiusError(f"Radius must be positive, got {radius}")
73-89: Suggest type consistency for --dilate argument.The argument parser and file handling logic are correct. The empty file list check (lines 83-85) properly validates input.
Optional refinement: Line 76 declares
--dilateastype=intwhileconvert_maskexpectsradius: float. Although Python's duck typing allows this to work, it creates a type inconsistency. For clarity, consider:- parser.add_argument("--dilate", type=int, default=1) + parser.add_argument("--dilate", type=float, default=1.0)This matches the function signature and allows fractional radius values (e.g.,
--dilate 1.5).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/dilate_segmentation_mask.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-15T11:26:16.124Z
Learnt from: Czaki
PR: 4DNucleome/PartSeg#1307
File: examples/dilate_segmentation_mask.py:0-0
Timestamp: 2025-10-15T11:26:16.124Z
Learning: In the PartSeg project, the minimum supported numpy version is 1.22.2, with a restriction to numpy<2 for Python versions below 3.10. Features from newer numpy versions (e.g., numpy 2.3) should be avoided to maintain backward compatibility.
Applied to files:
examples/dilate_segmentation_mask.py
🧬 Code graph analysis (1)
examples/dilate_segmentation_mask.py (5)
package/PartSegCore/image_operations.py (2)
dilate(96-105)to_binary_image(138-140)package/PartSegCore/mask/io_functions.py (4)
LoadROIImage(410-452)MaskProjectTuple(62-111)SaveROI(529-554)SaveROIOptions(114-125)package/PartSegCore/roi_info.py (1)
ROIInfo(33-124)package/PartSegCore/segmentation/watershed.py (2)
calculate_distances_array(281-297)get_neigh(268-269)package/PartSegImage/image.py (3)
dtype(461-463)spacing(773-775)fit_array_to_image(570-580)
🪛 Ruff (0.14.0)
examples/dilate_segmentation_mask.py
20-20: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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_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_linux)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- 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 (GetTestData linux)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
🔇 Additional comments (5)
examples/dilate_segmentation_mask.py (5)
1-16: LGTM! Imports are well-organized.All imports are relevant and necessary for ROI dilation functionality.
23-38: LGTM! Filtering and remapping logic is correct.The conditional filtering for
only_selectedmode properly:
- Masks non-selected components to zero
- Remaps remaining components to consecutive IDs (0, 1, 2, ...)
- Updates the selected_components list accordingly
This ensures the downstream
euclidean_sprawlfunction receives properly formatted input.
39-51: Verify that 2D-only dilation is intentional.Line 40 uses
dilate(bin_roi, [radius, radius], True)which dilates only in the XY plane (not Z) because:
- The radius is 2D:
[radius, radius]- The
layer=Trueparameter processes each 2D slice independentlyFor 3D volumetric images typical in microscopy, this means the ROI expands horizontally but not vertically (in Z). If full 3D dilation is needed, use
[radius, radius, radius]withlayer=False:- sprawl_area = dilate(bin_roi, [radius, radius], True) + sprawl_area = dilate(bin_roi, [radius, radius, radius], layer=False)If 2D-only dilation is intentional (e.g., for Z-stacks where vertical dilation is undesired), consider adding a comment to document this design choice.
Good catch on Line 41:
components_numnow correctly usesnp.max(roi_)from the filtered/remapped ROI rather than the original, avoiding the mismatch flagged in past reviews. Based on learnings.
52-70: LGTM! Saving logic is correct.The script properly:
- Constructs the output file path by inserting the suffix before the file extension
- Creates a
MaskProjectTuplewith the updated ROI and preserved metadata- Passes the
selected_componentslist (updated when filtering or original otherwise)- Uses appropriate
SaveROIOptionswith relative paths and mask data preservation
92-93: LGTM! Standard script entry point.The
if __name__ == "__main__"guard withsys.exit(main())is the correct idiom for executable Python scripts, ensuring the return code is properly propagated.
Czaki
force-pushed
the
add_usefull_scripts
branch
from
5964f72 to
50be8f4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
examples/dilate_segmentation_mask.py (2)
39-50: Confirm whether 2D-only dilation is intentional before sprawl
sprawl_area = dilate(bin_roi, [radius, radius], True)suggests dilation is applied only in X/Y withlayer=True(per-plane), even for 3D volumes. Given thatcalculate_distances_array(project.image.spacing, get_neigh(True))andeuclidean_sprawloperate in full 3D when spacing is 3D, this may under-dilate along Z.If the goal is isotropic 3D expansion, consider switching to a 3D radius and disabling per-layer mode, e.g.:
- sprawl_area = dilate(bin_roi, [radius, radius], True) + sprawl_area = dilate(bin_roi, [radius, radius, radius], layer=False)If instead you intentionally want per-slice 2D dilation, a short comment explaining that choice would make this behavior clearer to readers.
73-89: Align--dilateCLI type withradius: floatparameter
convert_maskdeclaresradius: float, but Line 76 defines--dilateastype=int. This works at runtime, but it’s inconsistent with the function signature and previous review feedback, and prevents passing non-integer radii if you ever support them.You can align the types (and slightly improve UX) by treating it as
float:def main(): parser = ArgumentParser() parser.add_argument("project_files", nargs="+", type=str) - parser.add_argument("--dilate", type=int, default=1) + parser.add_argument("--dilate", type=float, default=1.0) parser.add_argument("--suffix", type=str, default="_dilated") parser.add_argument("--only-selected", action="store_true") @@ for file_path in files: convert_mask(Path(file_path).absolute(), args.dilate, args.suffix, args.only_selected)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/dilate_segmentation_mask.py(1 hunks)examples/extract_components_from_project.py(1 hunks)examples/max_projection.py(1 hunks)package/PartSegImage/image_reader.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package/PartSegImage/image_reader.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-15T11:26:16.151Z
Learnt from: Czaki
Repo: 4DNucleome/PartSeg PR: 1307
File: examples/dilate_segmentation_mask.py:0-0
Timestamp: 2025-10-15T11:26:16.151Z
Learning: In the PartSeg project, the minimum supported numpy version is 1.22.2, with a restriction to numpy<2 for Python versions below 3.10. Features from newer numpy versions (e.g., numpy 2.3) should be avoided to maintain backward compatibility.
Applied to files:
examples/dilate_segmentation_mask.py
🧬 Code graph analysis (3)
examples/extract_components_from_project.py (1)
package/PartSegCore/mask/io_functions.py (3)
LoadROIImage(410-452)SaveComponents(617-651)SaveComponentsOptions(557-563)
examples/max_projection.py (3)
package/PartSegImage/image.py (2)
Image(198-1034)spacing(779-781)package/PartSegImage/image_writer.py (1)
ImageWriter(25-115)package/PartSegImage/image_reader.py (3)
TiffImageReader(461-683)read_image(168-189)read_image(268-289)
examples/dilate_segmentation_mask.py (5)
package/PartSegCore/image_operations.py (2)
dilate(96-105)to_binary_image(138-140)package/PartSegCore/mask/io_functions.py (4)
LoadROIImage(410-452)MaskProjectTuple(62-111)SaveROI(529-554)SaveROIOptions(114-125)package/PartSegCore/roi_info.py (1)
ROIInfo(33-124)package/PartSegCore/segmentation/watershed.py (2)
calculate_distances_array(281-297)get_neigh(268-269)package/PartSegImage/image.py (2)
spacing(779-781)fit_array_to_image(576-586)
🪛 GitHub Check: Codacy Static Code Analysis
examples/extract_components_from_project.py
[warning] 28-28: examples/extract_components_from_project.py#L28
Too many positional arguments for function call
🪛 Ruff (0.14.5)
examples/max_projection.py
20-20: Avoid specifying long messages outside the exception class
(TRY003)
examples/dilate_segmentation_mask.py
20-20: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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: Base py3.11 / macos-15-intel py 3.11 latest PyQt5
- GitHub Check: Base py3.11 / windows-latest py 3.11 latest PyQt5
- GitHub Check: Base py3.12 / ubuntu-22.04 py 3.12 latest PyQt6
- GitHub Check: Base py3.10 / ubuntu-22.04 py 3.10 latest PySide2
- GitHub Check: Base py3.12 / macos-15 py 3.12 latest PyQt6
- GitHub Check: Test PartSeg conda
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos_arm)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test windows)
- GitHub Check: 4DNucleome.PartSeg (Builds pyinstaller macos)
- GitHub Check: 4DNucleome.PartSeg (Builds sdist)
- GitHub Check: 4DNucleome.PartSeg (Tests_other test macos)
- 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 (Documentation_check help)
- GitHub Check: 4DNucleome.PartSeg (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check Notebook_check)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: Sourcery review
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
examples/max_projection.py (1)
13-45: Z max-projection with optional mask looks consistent with core APIsThe
max_projectionimplementation correctly:
- Validates presence of a
"Z"axis before indexing.- Uses
image.axis_order.index("Z")for the image data andimage.array_axis_order.index("Z")for the mask, which matches howImagearranges data vs. mask.- Builds a projected
Imagewithaxes_orderhaving"Z"removed andspacing=image.spacing[1:], consistent with the (Z, Y, X) spacing convention in the rest of the codebase.The CLI wrapper is straightforward and skips
_maskfiles appropriately when--with-maskis used.examples/dilate_segmentation_mask.py (2)
18-21: Radius validation is fine; Ruff TRY003 can likely be ignored hereThe
radius <= 0guard withValueError("Radius must be positive")is an appropriate early validation for a CLI script. The Ruff TRY003 warning about “long messages outside the exception class” is stylistic; for this small example script, the explicit message is clear and reasonable, so I would keep it as-is unless you want to tune Ruff settings for examples.
25-38: Component remapping and selected_components update look coherentThe
only_selectedbranch:
- Masks
roi_down toselected_components.- Uses
np.uniqueand a lookupmappingto remap labels to a dense 0..N range.- Resets
selected_componentstorange(1, len(unique_values))so it matches the new label IDs (skipping background 0).This keeps
components_num = np.max(roi_)in sync with the actual labels and avoids the earlier mismatch when using a filtered ROI.
|
Summary by Sourcery
Add two CLI example scripts for image processing: one to dilate segmentation masks and another to generate max projections of TIFF image stacks.
New Features:
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.