-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: set StructureSetToSegmentation continuous argument to False #77
base: main
Are you sure you want to change the base?
Conversation
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe pull request introduces a modification to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/readii/loaders.py (1)
Line range hint
75-92
: Consider adding debug logging for index-related issues.While the fix is solid, it might be helpful to add debug logging when indices are generated to help troubleshoot any future issues.
Add this logging before the StructureSetToSegmentation call:
logger.debug(f"Making mask using ROI names: {roiNames}") + logger.debug("Initializing StructureSetToSegmentation with continuous=False to ensure integer indices") # Initialize med-imagetools loader to convert RTSTRUCT point cloud to a segmentation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/readii/loaders.py
(1 hunks)
🔇 Additional comments (2)
src/readii/loaders.py (2)
Line range hint 82-92
: Existing error handling is sufficient for the change.
The try-except block properly handles ValueError, which would catch any issues during mask generation, including potential problems with the new continuous=False setting. The empty dictionary return on error provides a clean fallback.
75-79
: LGTM! The fix addresses the IndexError as intended.
The addition of continuous=False
is the correct solution to prevent float indices in the RTSTRUCT conversion, which was causing the IndexError with HNSCC data. The added comment clearly explains the rationale.
Let's verify the error handling for this change:
✅ Verification successful
The fix is properly integrated with error handling and safe to use
The code change is well-integrated within the existing error handling structure:
- The
StructureSetToSegmentation
is used only once in the codebase - The operation is wrapped in a try-except block that gracefully handles failures by returning an empty dictionary
- The
continuous=False
parameter will prevent float indices while maintaining the existing error recovery mechanism
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the error handling covers index-related issues
# Check for any other instances where StructureSetToSegmentation is used
rg "StructureSetToSegmentation" --type python
# Look for related error handling patterns
rg "IndexError" --type python
Length of output: 137
Script:
#!/bin/bash
# Let's try without the --type flag since it's not supported
# Check for any other instances where StructureSetToSegmentation is used
rg "StructureSetToSegmentation"
# Look for related error handling patterns
rg "IndexError"
# Also check for try-except blocks around this functionality
rg -A 5 "try:" src/readii/loaders.py
Length of output: 377
Script:
#!/bin/bash
# Let's check the full context around the try block and StructureSetToSegmentation usage
rg -B 5 -A 10 "StructureSetToSegmentation" src/readii/loaders.py
# Also check for any exception handling after the try block
rg -B 2 -A 5 "except" src/readii/loaders.py
Length of output: 1241
HNSCC data was generating the following error:
IndexError: [-65.00013333] index is out of bounds for image sized (123, 512, 512, 1).
This was caused by the med-imagetools class StructureSettoSegmentation initialization setting the continuous argument to True, which allowed for the RTSTRUCT physical points to be converted into indices that were floats instead of integers.
Setting continuous = False in loadRTSTRUCTSITK should solve this issue.
Summary by CodeRabbit