-
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: add image cropping preprocessing #119
Conversation
…gger and dictionary iteration
…for quick checking dimension match between image and centroid/mask/dimensions/etc.
…to centroid, and mask update in crop with pyradiomics
WalkthroughThe changes update a Jupyter notebook to modify image display settings and metadata, adjust the execution count, and change colormap and axis tick configurations. The Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant DIS as displayImageSlice
participant PLT as plt.subplots
Caller->>DIS: call displayImageSlice(image, sliceIdx, ..., ax)
alt ax provided
DIS->>DIS: Use provided axis
else ax is None
DIS->>PLT: Create new axis using plt.subplots()
end
DIS->>DIS: Display image using imshow with specified colormap
DIS->>Caller: Return the axis object
sequenceDiagram
participant Caller as Caller
participant CropFunc as crop_and_resize_image_and_mask
participant LabelCheck as LabelShapeStatisticsImageFilter
participant Processor as Image & Mask Processor
Caller->>CropFunc: call crop_and_resize_image_and_mask(image, mask, label, crop_method, resize_dimension)
CropFunc->>LabelCheck: Check if label exists in mask
alt Label found
CropFunc->>CropFunc: Determine crop box based on crop_method ("bounding_box", "centroid", "cube")
CropFunc->>Processor: Crop image and mask accordingly
alt Resize dimension provided
Processor->>Processor: Resize image (linear) and mask (nearest neighbor)
end
CropFunc->>Caller: Return cropped and resized image, mask
else Label not found
CropFunc->>Caller: Raise ValueError (label missing)
end
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
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 (
|
These have all been implemented in med-imagetools, use imports from there instead.
…ed from DICOM vs nifti
…nearest neighbour interpolation for mask resizing
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
🧹 Nitpick comments (6)
tests/process/images/test_crop.py (3)
2-2
: Remove unused import.The
SimpleITK
import is not directly used in this file.-import SimpleITK as sitk
🧰 Tools
🪛 Ruff (0.8.2)
2-2:
SimpleITK
imported but unusedRemove unused import:
SimpleITK
(F401)
9-16
: Make test data paths configurable.Hardcoded test data paths make tests brittle and less maintainable. Consider making these paths configurable through environment variables or test configuration.
Example implementation:
import os @pytest.fixture def test_data_dir(): return os.getenv('TEST_DATA_DIR', 'tests') @pytest.fixture def nsclcCT(test_data_dir): return os.path.join(test_data_dir, 'NSCLC_Radiogenomics/R01-001/09-06-1990-NA-CT_CHEST_ABD_PELVIS_WITH_CON-98785/3.000000-THORAX_1.0_B45f-95741')Also applies to: 19-21, 24-26
45-51
: Add docstring and explain expected size.The test function would benefit from a docstring explaining the default behavior being tested and why the expected size is (92, 92, 92).
def test_default_crop_and_resize_image(lung4D_image, lung4D_mask): + """Test the default cropping behavior using the cube method. + + The default crop method is 'cube', which expands the bounding box to a cube + with the maximum dimension. In this case, the maximum dimension is 92 voxels, + resulting in a (92, 92, 92) cube. + """ expected_size = (92, 92, 92)src/readii/process/images/crop.py (2)
67-69
: Use 'is' for None comparison.Replace
==
withis
when comparing withNone
as per Python style guidelines.- if resize_dimension == None: + if resize_dimension is None:🧰 Tools
🪛 Ruff (0.8.2)
67-67: Comparison to
None
should becond is None
Replace with
cond is None
(E711)
100-126
: Consider moving test code to a separate test file.The main block contains test code with hardcoded paths. Consider moving this code to a proper test file or using configurable paths.
src/readii/image_processing.py (1)
157-212
: Use 'is' for None comparisons.Replace
==
withis
when comparing withNone
as per Python style guidelines.- if dispMin == None: + if dispMin is None: - if dispMax == None: + if dispMax is None: - if ax == None: + if ax is None:🧰 Tools
🪛 Ruff (0.8.2)
187-187: Use
is
andis not
for type comparisons, orisinstance()
for isinstance checks(E721)
193-193: Comparison to
None
should becond is None
Replace with
cond is None
(E711)
195-195: Comparison to
None
should becond is None
Replace with
cond is None
(E711)
199-199: Multiple statements on one line (colon)
(E701)
200-200: Multiple statements on one line (colon)
(E701)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lock
is excluded by!**/*.lock
📒 Files selected for processing (6)
notebooks/viz_neg_controls.ipynb
(4 hunks)pyproject.toml
(1 hunks)src/readii/image_processing.py
(3 hunks)src/readii/process/images/__init__.py
(1 hunks)src/readii/process/images/crop.py
(1 hunks)tests/process/images/test_crop.py
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/readii/process/images/init.py
- pyproject.toml
🧰 Additional context used
🪛 Ruff (0.8.2)
tests/process/images/test_crop.py
2-2: SimpleITK
imported but unused
Remove unused import: SimpleITK
(F401)
src/readii/process/images/crop.py
67-67: Comparison to None
should be cond is None
Replace with cond is None
(E711)
🔇 Additional comments (5)
notebooks/viz_neg_controls.ipynb (3)
102-103
: LGTM! Good visualization improvements.The addition of the grayscale colormap and axis removal enhances the clarity of the donut image visualization.
106-108
: LGTM! Enhanced mask visualization.Good choice of using the 'Greens' colormap for the mask visualization and removing axis ticks for a cleaner display.
388-390
: Verify Python version compatibility.The notebook is using Python 3.12.8. Let's verify this version is officially released and stable.
Also applies to: 402-402
✅ Verification successful
🌐 Web query:
What is the latest stable version of Python 3.12?
💡 Result:
The latest stable version of Python 3.12 as of February 2025 is Python 3.12.8, released on December 3, 2024[5]. This maintenance release includes over 250 bug fixes, security updates like libexpat upgrades to address CVE-2024-50602[5], and documentation improvements since version 3.12.7.
Key details about the Python 3.12 series:
- Security focus: Recent versions prioritize patching vulnerabilities (e.g., CVE-2024-7592 fixed in v3.12.6)[3]
- Performance: Maintains a ~5% speed improvement over Python 3.11 through optimizations like BOLT binary enhancements[1][7]
- Support lifecycle: Security updates will continue through October 2028[4]
For most users, Python recommends upgrading to the latest stable version (currently v3.12.x) for security and stability benefits while waiting for newer major releases like Python 3.13[6].
Citations:
- 1: https://www.python.org/downloads/release/python-3125/
- 2: https://www.python.org/downloads/release/python-3123/
- 3: https://www.python.org/downloads/release/python-3126/
- 4: https://peps.python.org/pep-0693/
- 5: https://www.python.org/downloads/release/python-3128/
- 6: https://pythonspeed.com/articles/upgrade-python-3.13/
- 7: https://www.python.org/downloads/release/python-3120/
- 8: https://www.python.org/doc/versions/
- 9: https://realpython.com/python312-new-features/
- 10: https://docs.python.org/es/dev/whatsnew/3.12.html
Python Version Compatibility Confirmed
The web query confirms that Python 3.12.8 is the latest stable release as of February 2025. The notebook's usage of Python 3.12.8 is therefore compliant with the current stable version.tests/process/images/test_crop.py (1)
53-93
: LGTM! Comprehensive test coverage.The parameterized test is well-structured with good coverage of different cropping methods and resize dimensions. The test cases are clearly documented with comments explaining each scenario.
src/readii/image_processing.py (1)
157-212
: LGTM! Good improvement to function reusability.The addition of the optional
ax
parameter and returning the axis object improves the function's reusability by allowing it to be used in subplots and custom figure layouts.🧰 Tools
🪛 Ruff (0.8.2)
187-187: Use
is
andis not
for type comparisons, orisinstance()
for isinstance checks(E721)
193-193: Comparison to
None
should becond is None
Replace with
cond is None
(E711)
195-195: Comparison to
None
should becond is None
Replace with
cond is None
(E711)
199-199: Multiple statements on one line (colon)
(E701)
200-200: Multiple statements on one line (colon)
(E701)
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
🧹 Nitpick comments (2)
src/readii/process/images/crop.py (2)
30-31
: Fix parameter name in docstring.The parameter name in the docstring is
resize_dimensions
but the actual parameter isresize_dimension
(singular).- resize_dimensions : int, optional + resize_dimension : int, optional
106-110
: Consider using path constants or configuration for test paths.The hardcoded test paths could break if files are moved or renamed. Consider:
- Moving these paths to a configuration file
- Using path constants
- Using relative paths from a base test directory
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lock
is excluded by!**/*.lock
📒 Files selected for processing (1)
src/readii/process/images/crop.py
(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
src/readii/process/images/crop.py
67-67: Comparison to None
should be cond is None
Replace with cond is None
(E711)
🔇 Additional comments (2)
src/readii/process/images/crop.py (2)
1-9
: LGTM! Well-organized imports and clear type definitions.The imports are properly organized and the
CropMethods
type alias accurately reflects the implemented methods.
12-98
: LGTM! Well-implemented cropping functionality.The implementation is robust with:
- Comprehensive error handling
- Clear method-specific logic
- Proper image and mask resizing with appropriate interpolation methods
🧰 Tools
🪛 Ruff (0.8.2)
67-67: Comparison to
None
should becond is None
Replace with
cond is None
(E711)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #119 +/- ##
==========================================
+ Coverage 48.12% 50.25% +2.13%
==========================================
Files 34 36 +2
Lines 1492 1546 +54
==========================================
+ Hits 718 777 +59
+ Misses 774 769 -5 ☔ View full report in Codecov by Sentry. |
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.
looks solid
Using crop methods from med-imagetools, setup three crop methods that can be used as preprocessing steps for feature extraction.
Three methods migrated from readii-fmcib = bbox, centroid, cube
Summary by CodeRabbit