-
Notifications
You must be signed in to change notification settings - Fork 9
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
fix(datagraph): iteration through edge_types wasnt updated for seg #131
base: main
Are you sure you want to change the base?
Conversation
…EG->CT/MR mapping in final dataframe processing
WalkthroughThe changes involve modifications to the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)
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: 1
🧹 Outside diff range and nitpick comments (1)
src/imgtools/modules/datagraph.py (1)
317-333
: Consider improving error messages for unsupported modality combinations.When an unsupported modality combination is provided, the error message "Please enter the correct query" could be more helpful by listing the supported combinations, especially for the newly added SEG modality.
- raise ValueError("Please enter the correct query") + raise ValueError( + "Unsupported modality combination. Supported combinations:\n" + "- Single modality: CT, MR, RTSTRUCT, RTDOSE, PT, SEG\n" + "- Two modalities: CT/MR,SEG, CT/MR,RTSTRUCT, etc.\n" + "- Three or more: CT/MR,RTSTRUCT,RTDOSE, etc." + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/imgtools/modules/datagraph.py (2 hunks)
🔇 Additional comments (2)
src/imgtools/modules/datagraph.py (2)
317-333
: LGTM: SEG modality handling is correctly implemented.The implementation for handling SEG->CT/MR connections is consistent and properly renames the columns for the destination and origin nodes.
Line range hint
1-1
: Verify SEG modality implementation consistency.Let's verify that all SEG-related changes are consistent throughout the codebase.
✅ Verification successful
Based on the shell script results, I can now provide a final response regarding the SEG modality implementation consistency:
SEG modality implementation is properly integrated and consistent.
The codebase shows proper SEG modality support with consistent implementation:
- SEG modality is correctly handled in the loader system (src/imgtools/io/loaders.py) with a dedicated
read_dicom_seg()
function- SEG reference handling is properly implemented in the crawler (src/imgtools/utils/crawl.py) with specific logic for SEG metadata extraction
- The implementation follows the same pattern as other modalities (CT, MR, RTSTRUCT, etc.)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify SEG modality implementation consistency # Test 1: Check for SEG modality references echo "Checking SEG modality references..." rg -l "SEG" --type py # Test 2: Check for edge type 7 references echo "Checking edge type 7 references..." rg "edge.*7" --type py # Test 3: Check for SEG in modality lists echo "Checking SEG in modality lists..." rg "supp_mods.*=.*\[.*\]" --type pyLength of output: 376
Script:
#!/bin/bash # Let's get more context about SEG implementation # Check SEG references with context echo "SEG references with context:" rg "SEG" --type py -B 2 -A 2 # Check modality-related configurations or lists echo "Modality configurations:" rg "modalities|MODALITIES|supported_mod" --type py -B 1 -A 1 # Check edge type definitions and handling echo "Edge type handling:" rg "edge_type|EDGE_TYPE" --type py -B 1 -A 1 # Check specific file content where SEG was found echo "Content of files with SEG:" cat src/imgtools/utils/crawl.py echo "---" cat src/imgtools/io/loaders.pyLength of output: 41039
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #131 +/- ##
==========================================
+ Coverage 45.69% 45.73% +0.04%
==========================================
Files 30 30
Lines 2867 2871 +4
==========================================
+ Hits 1310 1313 +3
- Misses 1557 1558 +1 ☔ View full report in Codecov by Sentry. |
update edge_types to 8
handle new edge type for SEG->CT/MR mapping in final dataframe processing in parser
Summary by CodeRabbit
New Features
SEG
andCT/MR
.Bug Fixes
Chores
attrs
to allow broader version compatibility.