fix: Clean elements outside mask for multiple otsu method#1319
fix: Clean elements outside mask for multiple otsu method#1319
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures that segmentation outputs respect the provided mask by zeroing out elements outside it before further processing in the multiple OTUs method. Sequence diagram for segmentation output masking in multiple OTUs methodsequenceDiagram
participant "calculation_run()"
participant "SimpleITK"
participant "Mask"
participant "Result Array"
"calculation_run()"->>"SimpleITK": GetArrayFromImage(res)
"SimpleITK"-->>"Result Array": Return array
"calculation_run()"->>"Mask": Check if mask exists
alt Mask exists
"calculation_run()"->>"Result Array": Set elements outside mask to 0
end
"calculation_run()"->>"Result Array": Continue processing (bincount, etc.)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughAdded a dependency entry "otsu" to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant I as Image + Mask
participant A as RestartableSegmentation
participant ITK as ITK Engine
participant NP as NumPy Ops
I->>A: Provide image and mask
A->>ITK: Run segmentation
ITK-->>A: Return ITK label image
A->>NP: Convert ITK label image → NumPy `res`
A->>NP: Apply mask: `res[mask==0] = 0` %% New/changed step (mask enforced)
A->>NP: Compute sizes (`bincount`)
A->>A: Generate thresholds / annotations based on masked `res`
A-->>I: Return segmentation labels and metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package/PartSegCore/segmentation/restartable_segmentation_algorithms.py (1)
645-646: Consider adding test coverage for mask application.Ensure test coverage verifies that:
- Segmentation voxels outside the mask are properly zeroed
- Size counts exclude masked regions
- The behavior is correct when mask is None vs. when mask is provided
Would you like me to help draft unit tests for this masking behavior?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/project_dict.pws(1 hunks)package/PartSegCore/segmentation/restartable_segmentation_algorithms.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
package/PartSegCore/segmentation/restartable_segmentation_algorithms.py (4)
package/PartSeg/common_gui/algorithms_description.py (1)
mask(685-686)package/PartSeg/common_backend/base_settings.py (2)
mask(582-583)mask(586-591)package/PartSegImage/image.py (1)
mask(539-540)package/PartSegCore/segmentation/algorithm_base.py (2)
mask(165-168)mask(171-175)
🪛 GitHub Actions: Tests
package/PartSegCore/segmentation/restartable_segmentation_algorithms.py
[error] 645-645: ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all().
⏰ 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: 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: 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: 4DNucleome.PartSeg (manifest_check manifest_check)
- GitHub Check: Sourcery review
🔇 Additional comments (2)
.github/project_dict.pws (1)
27-27: LGTM!Adding "otsu" to the spell-check dictionary is appropriate for this codebase that implements Otsu thresholding algorithms.
package/PartSegCore/segmentation/restartable_segmentation_algorithms.py (1)
645-646: Only OtsuSegment needs explicit mask‐zeroing; other threshold algorithms usethr.calculate_mask(which respectsself.mask) and don’t convert ITK images directly to NumPy.
package/PartSegCore/segmentation/restartable_segmentation_algorithms.py
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package/PartSegCore/segmentation/restartable_segmentation_algorithms.py (1)
645-646: LGTM! The masking logic is correctly implemented.The fix from the previous review has been applied—using
if self.mask is not None:instead ofif self.mask:correctly handles NumPy arrays. The masking operation (res[self.mask == 0] = 0) appropriately zeroes out segmentation values outside the mask before size counting and annotation generation.This approach is sound because
SimpleITK.OtsuMultipleThresholdsdoesn't accept a mask parameter, so post-processing the result is the correct way to constrain segmentation to the masked region.Optional: Consider adding a clarifying comment.
For future maintainability, you might add a brief comment explaining why masking is applied here:
# Apply mask since SimpleITK.OtsuMultipleThresholds doesn't support masking if self.mask is not None: res[self.mask == 0] = 0
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package/PartSegCore/segmentation/restartable_segmentation_algorithms.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
package/PartSegCore/segmentation/restartable_segmentation_algorithms.py (3)
package/PartSeg/common_gui/algorithms_description.py (1)
mask(685-686)package/PartSegImage/image.py (1)
mask(539-540)package/PartSegCore/segmentation/algorithm_base.py (2)
mask(165-168)mask(171-175)
⏰ 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). (18)
- 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: Base py3.11 / macos-15-intel py 3.11 latest PyQt5
- GitHub Check: Base py3.10 / ubuntu-24.04 py 3.10 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 (formatting_check check_formating)
- GitHub Check: 4DNucleome.PartSeg (GetTestData linux)
- GitHub Check: 4DNucleome.PartSeg (Documentation_check help)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Sourcery review
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1319 +/- ##
========================================
Coverage 93.16% 93.16%
========================================
Files 210 210
Lines 33285 33287 +2
========================================
+ Hits 31009 31011 +2
Misses 2276 2276 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Czaki
changed the title
1 participant
Fix problem from image:
Summary by Sourcery
Bug Fixes:
Summary by CodeRabbit
Bug Fixes
Chores