fix: look for image type in Path stem as Path is not iterable #97
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 modifies 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/readii/io/loaders/features.py (1)
Line range hint
58-71: Improve clarity and control in the pattern matching logicThe current implementation handles both single and multiple matches the same way, with only a warning for multiple matches. This could lead to unexpected behavior.
Consider making the logic more explicit:
match len(matching_files): case 1: - # Only one file found, use it - pass + # Only one file found, use it + image_type_feature_file = matching_files[0] case 0: # No files found for this image type logger.warning(f"No {image_type} feature csv files found in {extracted_feature_dir}") # Skip to the next image type continue case _: # Multiple files found msg = f"Multiple {image_type} feature csv files found in {extracted_feature_dir}. First one will be used." logger.warning(msg) + # Consider if this should be an error instead of a warning + image_type_feature_file = matching_files[0] -image_type_feature_file = matching_files[0]
🧹 Nitpick comments (1)
src/readii/io/loaders/features.py (1)
56-56: Consider using more precise string matching for image typesWhile using
Path.stemfixes the Path iteration issue, the current string containment check (in) could lead to false matches if image types are substrings of each other (e.g., "original" would match "original_filtered").Consider using more precise matching:
-matching_files = [file for file in feature_file_list if (image_type in file.stem)] +matching_files = [file for file in feature_file_list if any( + file.stem == image_type or + file.stem.startswith(f"{image_type}_") or + file.stem.endswith(f"_{image_type}") +)]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/readii/io/loaders/features.py(1 hunks)
🔇 Additional comments (1)
src/readii/io/loaders/features.py (1)
Line range hint 56-71: Verify the impact on existing image type patterns
Let's verify how this function is used in the codebase and what image types are typically processed.
✅ Verification successful
Changes to image type handling are safe and well-supported
The verification shows that the changes to handle multiple matching files are safe because:
- The image types are well-defined in configuration files (
NSCLC_Radiogenomics.yamland4D-Lung.yaml) with specific patterns like "original", "shuffled_full", etc. - The warning message for multiple matches is appropriate as it maintains backward compatibility by using the first match, while informing users about potential issues.
- The function is properly tested and integrated with the dataset configurations, as seen in the test files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage patterns of loadFeatureFilesFromImageTypes and common image types
# Check how the function is called and what image types are passed
rg -A 5 "loadFeatureFilesFromImageTypes" --glob '!*.csv'
# Look for test files with image type patterns
fd -e csv . | grep -i "feature"
# Check for any test files that might be affected
fd -e py -e ipynb | xargs rg "image_type.*original"
Length of output: 1427
Script:
#!/bin/bash
# Let's try a different approach to understand the usage and impact
# Check for any direct usage of this function in the codebase
rg -l "loadFeatureFilesFromImageTypes\("
# Look for patterns of image types being defined or used
rg "image_types.*=.*\[" -A 2
# Check the entire features.py file to understand the context better
cat src/readii/io/loaders/features.py
# Look for test files related to features
fd "test.*feature" -t f
Length of output: 4982
Script:
#!/bin/bash
# Let's check the test file and any other related usage patterns
# Check the content of the test file
cat tests/test_feature_extraction.py
# Look for any image type definitions in the test files
rg -g "test_*.py" "image_type" -B 2 -A 2
# Check for any configuration files that might define image types
fd -e yaml -e json -e toml | xargs rg "image.*type"
# Look for any notebooks that might use this function
fd -e ipynb | grep -v checkpoint | xargs rg "loadFeatureFilesFromImageTypes"
Length of output: 10642
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #97 +/- ##
=======================================
Coverage 41.66% 41.66%
=======================================
Files 33 33
Lines 1452 1452
=======================================
Hits 605 605
Misses 847 847 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
New Features
Bug Fixes