fix: pattern resolver update #116
Conversation
|
Warning Rate limit exceeded@strixy16 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 18 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request includes updates to parameter names and data structures in various test files and the Changes
Possibly related PRs
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 (
|
|
oops sorry about that, forgot it was used here |
updated loadSegmentationRTSTRUCT to use a dictionary input for roiNames since med-imagetools updated the functionality and lists and strings don't work as expected anymore
![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
🔭 Outside diff range comments (1)
tests/test_feature_extraction.py (1)
Line range hint
134-139: Update roiNames parameter in test_4DLung_radiomicFeatureExtraction_output.This test still uses the old string format for roiNames while other tests have been updated to use the dictionary format.
Apply this diff to maintain consistency:
actual = radiomicFeatureExtraction(lung4DMetadataPath, imageDirPath = "tests/", - roiNames = "Tumor_c40", + roiNames = {"GTV": "Tumor_c.*"}, outputDirPath = "tests/4D-Lung/results/")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
tests/test_feature_extraction.py(1 hunks)tests/test_image_processing.py(1 hunks)tests/test_loaders.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Unit-Tests (windows-latest, py312)
- GitHub Check: Unit-Tests (windows-latest, py311)
- GitHub Check: Unit-Tests (windows-latest, py310)
🔇 Additional comments (3)
tests/test_image_processing.py (1)
21-22: LGTM! The changes align with the standardization of ROI naming.The update from string pattern to dictionary format provides better structure and clarity for ROI specification.
Let's verify that this change is consistent across all test files:
✅ Verification successful
The ROI naming standardization is consistent with the test suite
The change to dictionary format
{'GTV':'Tumor_c.*'}aligns with the established pattern intest_loaders.pyand maintains compatibility with the loader implementation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all roiNames parameter usage rg "roiNames\s*=" -A 2Length of output: 3754
Script:
#!/bin/bash # Check the implementation of loadSegmentation and related functions ast-grep --pattern 'def loadSegmentation($$$)' -A 30Length of output: 54
Script:
#!/bin/bash # Search for segmentation loading related functions with more context rg "def.*[Ss]egmentation|def.*RTSTRUCT" -A 30Length of output: 34686
tests/test_loaders.py (1)
61-61: LGTM! Consistent with the ROI naming standardization.The changes align with the updates in other test files, using the dictionary format for ROI specification and the 'GTV' key.
Also applies to: 68-68
tests/test_feature_extraction.py (1)
40-41: LGTM! Consistent with ROI naming standardization.The changes align with the updates in other test files.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/test_loaders.py (1)
74-75:⚠️ Potential issueFix hardcoded error message in assertion.
The error message is hardcoded to "GTV" but the test handles multiple expected values ("GTV" and "Tumor_c40").
Apply this diff to make the error message dynamic:
assert list(actual.keys()) == [expected], \ - f"Segmentation label is wrong, should be GTV, getting {list(actual.keys())}" + f"Segmentation label is wrong, should be {expected}, getting {list(actual.keys())}"
🧹 Nitpick comments (1)
tests/test_loaders.py (1)
Line range hint
1-1: Consider using explicit imports instead of star imports.While star imports are common in test files, explicit imports make the code more maintainable and help static analyzers. Consider importing the required functions explicitly:
-from readii.loaders import * +from readii.loaders import loadDicomSITK, loadSegmentation🧰 Tools
🪛 Ruff (0.8.2)
67-67:
loadSegmentationmay be undefined, or defined from star imports(F405)
79-79:
sitkmay be undefined, or defined from star imports(F405)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
src/readii/process/__init__.py(1 hunks)tests/test_loaders.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
tests/test_loaders.py
67-67: loadSegmentation may be undefined, or defined from star imports
(F405)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Unit-Tests (macos-14, py312)
- GitHub Check: Unit-Tests (macos-14, py311)
- GitHub Check: Unit-Tests (macos-14, py310)
- GitHub Check: Unit-Tests (macos-latest, py311)
- GitHub Check: Unit-Tests (macos-latest, py310)
🔇 Additional comments (2)
src/readii/process/__init__.py (1)
16-20: Verify that the functions exist in the new.subsetmodule.The functions have been moved from
.selectto.subset. Let's verify that they exist in the new location.✅ Verification successful
Functions successfully relocated to
.subsetmoduleAll three functions are properly defined in the new location with their complete implementations, and no legacy imports from
.selectremain in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the functions exist in the new .subset module # Search for function definitions in the .subset module echo "Searching for function definitions in .subset module..." rg -A 5 "^def (dropUpToFeature|getOnlyPyradiomicsFeatures|selectByColumnValue)" src/readii/process/subset.py # Search for any remaining usages of these functions from .select echo "Checking for any remaining imports from .select..." rg "from.*\.select.*import.*(dropUpToFeature|getOnlyPyradiomicsFeatures|selectByColumnValue)"Length of output: 1588
tests/test_loaders.py (1)
57-64: Well-structured test parametrization!The parametrization effectively covers multiple input formats while maintaining backward compatibility. This is a good example of comprehensive test design.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #116 +/- ##
=======================================
Coverage 48.12% 48.12%
=======================================
Files 34 34
Lines 1492 1492
=======================================
Hits 718 718
Misses 774 774 ☔ View full report in Codecov by Sentry. |
PatternResolverinmed-imagetoolsupdated the input argument frompattern_parsertopattern_matcher, updated this to match here.Summary by CodeRabbit
PatternResolverclass for improved clarity and consistency.roiNamesparameter format in segmentation loading tests for better structure and clarity.