Add clinical preprocessing utilities for CT and MRI modalities. #8659
Conversation
…nd example notebooks
for more information, see https://pre-commit.ci
Hitendrasinhdata7
requested review from
KumoLiu,
Nic-Ma and
ericspod
as code owners
WalkthroughAdds monai/transforms/clinical_preprocessing.py providing ModalityTypeError and UnsupportedModalityError, CT and MRI pipelines via get_ct_preprocessing_pipeline and get_mri_preprocessing_pipeline, and API functions preprocess_medical_image and preprocess_dicom_series that validate modality and apply the appropriate pipeline. CT pipeline composes LoadImage(image_only=True), EnsureChannelFirst, ScaleIntensityRange(-1000, 400, 0.0, 1.0, clip=True); MRI pipeline composes LoadImage(image_only=True), EnsureChannelFirst, NormalizeIntensity(nonzero=True). Adds tests in monai/tests/test_clinical_preprocessing.py and re-exports new symbols from monai/transforms/init.py. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (1)
monai/transforms/clinical_preprocessing.py (1)
14-14: Consider using a tuple for SUPPORTED_MODALITIES.The current string constant works for error messages, but a tuple would better represent a collection of discrete values and allow programmatic iteration.
-SUPPORTED_MODALITIES = "CT, MR, MRI" +SUPPORTED_MODALITIES = ("CT", "MR", "MRI")Then update line 89 to format it:
- raise ValueError(f"Unsupported modality: {modality}. Supported values: {SUPPORTED_MODALITIES}") + raise ValueError(f"Unsupported modality: {modality}. Supported values: {', '.join(SUPPORTED_MODALITIES)}")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (2)
docs/clinical_dicom_workflow.pdfis excluded by!**/*.pdfmonai/docs/clinical_dicom_workflow.pdfis excluded by!**/*.pdf
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/transforms/clinical_preprocessing.pymonai/tests/test_clinical_preprocessing.py
🧬 Code graph analysis (2)
monai/transforms/clinical_preprocessing.py (4)
monai/transforms/compose.py (1)
Compose(123-393)monai/transforms/io/array.py (1)
LoadImage(109-305)monai/transforms/utility/array.py (1)
EnsureChannelFirst(175-234)monai/transforms/intensity/array.py (2)
ScaleIntensityRange(959-1013)NormalizeIntensity(816-925)
monai/tests/test_clinical_preprocessing.py (2)
monai/transforms/intensity/array.py (2)
ScaleIntensityRange(959-1013)NormalizeIntensity(816-925)monai/transforms/clinical_preprocessing.py (3)
get_ct_preprocessing_pipeline(17-39)get_mri_preprocessing_pipeline(42-58)preprocess_dicom_series(61-91)
🪛 Ruff (0.14.8)
monai/transforms/clinical_preprocessing.py
64-64: Undefined name MetaTensor
(F821)
80-80: Avoid specifying long messages outside the exception class
(TRY003)
89-89: Avoid specifying long messages outside the exception class
(TRY003)
monai/tests/test_clinical_preprocessing.py
41-41: Unused function argument: mock_loadimage
(ARG001)
51-51: Unused function argument: mock_loadimage
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: build-docs
- GitHub Check: packaging
🔇 Additional comments (11)
monai/tests/test_clinical_preprocessing.py (8)
1-13: LGTM!Imports and module structure are appropriate for testing clinical preprocessing utilities.
15-26: LGTM!Direct transform test correctly validates CT windowing behavior with appropriate tolerances.
28-38: LGTM!Direct transform test correctly validates MRI normalization with appropriate statistical tolerances.
40-48: LGTM!Pipeline construction test correctly validates the transform sequence. The
mock_loadimageparameter flagged by static analysis is required by the@patchdecorator.
50-58: LGTM!Pipeline construction test correctly validates the MRI transform sequence.
60-98: LGTM!Modality dispatch tests comprehensively validate CT and MRI pipeline selection, including case-insensitive handling and modality aliases.
100-120: LGTM!Error handling tests thoroughly validate TypeError and ValueError cases with appropriate message checks.
122-129: LGTM!Whitespace handling test validates input normalization for robust modality parsing.
monai/transforms/clinical_preprocessing.py (3)
17-39: LGTM!CT preprocessing pipeline correctly applies standard HU windowing [-1000, 400] mapped to [0.0, 1.0]. Docstring is clear and complete.
42-58: LGTM!MRI preprocessing pipeline correctly applies intensity normalization with
nonzero=Trueto handle zero-valued background. Docstring is clear and complete.
61-91: LGTM!Dispatcher function correctly validates input type, normalizes modality string, and selects appropriate pipeline. Error messages are informative. Docstring is complete with all sections.
…ew: add MetaTensor type hint, tuple for SUPPORTED_MODALITIES, and improved error messages
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
monai/transforms/clinical_preprocessing.py (1)
81-93: Consider extracting error messages to constants or custom exceptions.Static analysis flags the inline error messages. While the current approach is functional, extracting them improves maintainability.
+class UnsupportedModalityError(ValueError): + """Raised when an unsupported modality is provided.""" + pass + +class ModalityTypeError(TypeError): + """Raised when modality is not a string.""" + pass + def preprocess_dicom_series( dicom_path: Union[str, bytes, PathLike], modality: str, ) -> MetaTensor: # ... docstring ... if not isinstance(modality, str): - raise TypeError(f"modality must be a string, got {type(modality).__name__}") + raise ModalityTypeError(f"modality must be a string, got {type(modality).__name__}") modality = modality.strip().upper() if modality == "CT": transform = get_ct_preprocessing_pipeline() elif modality in ("MR", "MRI"): transform = get_mri_preprocessing_pipeline() else: - raise ValueError( - f"Unsupported modality: {modality}. Supported values: {', '.join(SUPPORTED_MODALITIES)}" - ) + raise UnsupportedModalityError( + f"Unsupported modality: {modality}. Supported values: {', '.join(SUPPORTED_MODALITIES)}" + )monai/tests/test_clinical_preprocessing.py (1)
56-75: Use isinstance checks instead of string comparison.Comparing class names with strings is brittle if classes are renamed or aliased.
Apply this diff for both pipeline tests:
def test_ct_preprocessing_pipeline(): """Test CT preprocessing pipeline returns expected transform composition.""" + from monai.transforms import LoadImage, EnsureChannelFirst, ScaleIntensityRange pipeline = get_ct_preprocessing_pipeline() assert hasattr(pipeline, 'transforms') assert len(pipeline.transforms) == 3 - assert pipeline.transforms[0].__class__.__name__ == 'LoadImage' - assert pipeline.transforms[1].__class__.__name__ == 'EnsureChannelFirst' - assert pipeline.transforms[2].__class__.__name__ == 'ScaleIntensityRange' + assert isinstance(pipeline.transforms[0], LoadImage) + assert isinstance(pipeline.transforms[1], EnsureChannelFirst) + assert isinstance(pipeline.transforms[2], ScaleIntensityRange) def test_mri_preprocessing_pipeline(): """Test MRI preprocessing pipeline returns expected transform composition.""" + from monai.transforms import LoadImage, EnsureChannelFirst, NormalizeIntensity pipeline = get_mri_preprocessing_pipeline() assert hasattr(pipeline, 'transforms') assert len(pipeline.transforms) == 3 - assert pipeline.transforms[0].__class__.__name__ == 'LoadImage' - assert pipeline.transforms[1].__class__.__name__ == 'EnsureChannelFirst' - assert pipeline.transforms[2].__class__.__name__ == 'NormalizeIntensity' + assert isinstance(pipeline.transforms[0], LoadImage) + assert isinstance(pipeline.transforms[1], EnsureChannelFirst) + assert isinstance(pipeline.transforms[2], NormalizeIntensity)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/tests/test_clinical_preprocessing.pymonai/transforms/clinical_preprocessing.py
🧬 Code graph analysis (1)
monai/tests/test_clinical_preprocessing.py (1)
monai/transforms/clinical_preprocessing.py (3)
get_ct_preprocessing_pipeline(19-41)get_mri_preprocessing_pipeline(44-60)preprocess_dicom_series(63-95)
🪛 Ruff (0.14.8)
monai/tests/test_clinical_preprocessing.py
130-130: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
142-142: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
144-144: Local variable error_message is assigned to but never used
Remove assignment to unused variable error_message
(F841)
monai/transforms/clinical_preprocessing.py
82-82: Avoid specifying long messages outside the exception class
(TRY003)
91-93: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (6)
monai/transforms/clinical_preprocessing.py (3)
1-16: LGTM!Module structure is clean. MetaTensor is properly imported, addressing the previous review comment. Using a tuple for
SUPPORTED_MODALITIESis appropriate.
19-41: CT preprocessing pipeline is well-configured.The HU window [-1000, 400] is standard for soft tissue imaging. Including
clip=Trueprevents out-of-range values. Docstring is complete and follows Google style.
44-60: MRI preprocessing pipeline is appropriate.Using
NormalizeIntensity(nonzero=True)correctly handles background voxels in MRI. Docstring is complete and follows Google style.monai/tests/test_clinical_preprocessing.py (3)
12-34: Direct transform test is thorough.Testing ScaleIntensityRange with synthetic data validates the windowing logic. Shape preservation, finiteness, and range checks with tolerance are appropriate.
37-53: Direct normalization test is sound.Testing NormalizeIntensity validates the statistical properties. Tolerance values (atol=0.1) are reasonable for normalized data.
78-124: Dispatcher tests with mocks are well-structured.Good coverage of case-insensitive modality handling (CT, ct, MRI, MR). Mocks appropriately isolate the dispatcher logic.
…e checks in tests, and improve error handling
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
monai/tests/test_clinical_preprocessing.py (2)
12-19: Consider verifying transform parameters.Test checks composition but not ScaleIntensityRange parameters. Add assertions for
a_min,a_max,b_min,b_max,clipto catch parameter regressions.def test_ct_preprocessing_pipeline(): """Test CT preprocessing pipeline returns expected transform composition.""" pipeline = get_ct_preprocessing_pipeline() assert hasattr(pipeline, 'transforms') assert len(pipeline.transforms) == 3 assert isinstance(pipeline.transforms[0], LoadImage) assert isinstance(pipeline.transforms[1], EnsureChannelFirst) assert isinstance(pipeline.transforms[2], ScaleIntensityRange) + # Verify CT-specific HU window parameters + scale_transform = pipeline.transforms[2] + assert scale_transform.a_min == -1000 + assert scale_transform.a_max == 400 + assert scale_transform.b_min == 0.0 + assert scale_transform.b_max == 1.0 + assert scale_transform.clip is True
22-29: Consider verifying transform parameters.Test checks composition but not NormalizeIntensity parameters. Add assertion for
nonzeroparameter.def test_mri_preprocessing_pipeline(): """Test MRI preprocessing pipeline returns expected transform composition.""" pipeline = get_mri_preprocessing_pipeline() assert hasattr(pipeline, 'transforms') assert len(pipeline.transforms) == 3 assert isinstance(pipeline.transforms[0], LoadImage) assert isinstance(pipeline.transforms[1], EnsureChannelFirst) assert isinstance(pipeline.transforms[2], NormalizeIntensity) + # Verify MRI-specific normalization parameter + normalize_transform = pipeline.transforms[2] + assert normalize_transform.nonzero is True
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/tests/test_clinical_preprocessing.pymonai/transforms/clinical_preprocessing.py
🧬 Code graph analysis (1)
monai/tests/test_clinical_preprocessing.py (1)
monai/transforms/clinical_preprocessing.py (5)
get_ct_preprocessing_pipeline(28-34)get_mri_preprocessing_pipeline(37-43)preprocess_dicom_series(46-77)UnsupportedModalityError(18-20)ModalityTypeError(23-25)
🪛 Ruff (0.14.8)
monai/transforms/clinical_preprocessing.py
64-64: Avoid specifying long messages outside the exception class
(TRY003)
73-75: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: packaging
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: build-docs
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-py3 (3.11)
🔇 Additional comments (7)
monai/tests/test_clinical_preprocessing.py (3)
1-9: Imports look good.All necessary imports present for current tests.
32-35: LGTM - properly uses pytest.raises.Error handling test correctly implemented per previous feedback.
38-41: LGTM - properly uses pytest.raises.Type error handling test correctly implemented per previous feedback.
monai/transforms/clinical_preprocessing.py (4)
1-15: Module setup looks good.Imports and constants properly defined. MetaTensor import addresses previous review feedback.
17-25: Custom exceptions properly defined.Simple exception classes with descriptive docstrings.
46-62: Well-documented function signature.Complete Google-style docstring with Args, Returns, and Raises sections. Type hints properly defined.
63-77: Implementation logic is correct.Type checking, normalization, and modality dispatch work as expected. Transform application delegates file validation to LoadImage.
Static analysis flags TRY003 (inline exception messages), but this is common practice for descriptive errors and can be ignored.
…ecks, and full tests with successful execution
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
monai/tests/test_clinical_preprocessing.py (2)
13-43: Pipeline assertions are OK; consider tightening a couple parameter checks to reduce false positives.
Right now you only validate transform types (and for CT/MRI only one parameter each). If these pipelines are meant to be stable APIs, consider also asserting key ctor args, e.g.LoadImage(image_only=True)and (if relevant)EnsureChannelFirstconfiguration, so a future change doesn’t silently degrade behavior while keeping the same classes.
45-54: Exception tests look good; message match is a bit brittle to ordering.
If the implementation ever changes the presentation order of supported modalities (e.g., using a set), the regexCT, MR, MRIwill fail despite correct behavior—consider matching the tokens independently (or matching againstSUPPORTED_MODALITIES-derived formatting if exposed).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/transforms/clinical_preprocessing.pymonai/tests/test_clinical_preprocessing.py
🪛 Ruff (0.14.8)
monai/tests/test_clinical_preprocessing.py
65-65: Unused lambda argument: x
(ARG005)
78-78: Unused lambda argument: x
(ARG005)
- Remove test code from production module and implement proper preprocessing pipelines - Add custom exception classes (ModalityTypeError, UnsupportedModalityError) - Fix test file mocking to avoid Ruff ARG005 lint warnings (use Mock instead of lambda) - Add verification of key constructor arguments (LoadImage configuration) - Make error message matching robust to ordering changes - Add comprehensive docstrings and input validation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
monai/tests/test_clinical_preprocessing.py (1)
53-64: Weak assertion withoroperator.The error message format guarantees both "PET" and "Unsupported modality" are present. Using
ormakes this assertion pass even if one is missing.- assert "PET" in error_message or "Unsupported modality" in error_message + assert "Unsupported modality" in error_message + assert "PET" in error_message
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/tests/test_clinical_preprocessing.pymonai/transforms/clinical_preprocessing.py
🧬 Code graph analysis (2)
monai/tests/test_clinical_preprocessing.py (1)
monai/transforms/clinical_preprocessing.py (5)
get_ct_preprocessing_pipeline(21-40)get_mri_preprocessing_pipeline(43-62)preprocess_dicom_series(65-104)UnsupportedModalityError(16-18)ModalityTypeError(11-13)
monai/transforms/clinical_preprocessing.py (4)
monai/transforms/compose.py (1)
Compose(123-393)monai/transforms/io/array.py (1)
LoadImage(109-305)monai/transforms/utility/array.py (1)
EnsureChannelFirst(175-234)monai/transforms/intensity/array.py (2)
ScaleIntensityRange(959-1013)NormalizeIntensity(816-925)
🪛 Ruff (0.14.8)
monai/transforms/clinical_preprocessing.py
83-83: Avoid specifying long messages outside the exception class
(TRY003)
99-101: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: build-docs
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: packaging
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-os (macOS-latest)
🔇 Additional comments (6)
monai/transforms/clinical_preprocessing.py (3)
21-40: Pipeline implementation looks correct.CT HU window [-1000, 400] is appropriate for soft tissue. Docstring is complete with Returns and Note sections.
43-62: MRI pipeline implementation looks correct.NormalizeIntensity with
nonzero=Trueproperly handles background regions. Docstring follows Google style.
81-104: Logic is sound.Modality normalization (strip, uppercase, MRI→MR alias) and error handling are well implemented. The TRY003 static analysis hints about exception message length are stylistic preferences; current approach is clear and readable.
monai/tests/test_clinical_preprocessing.py (3)
13-32: Thorough pipeline composition test.Verifies transform types, HU window parameters, and LoadImage configuration.
35-50: Good MRI pipeline coverage.Validates transform composition and nonzero normalization parameter.
77-101: Mock-based tests cover modality variants well.Tests uppercase, lowercase, whitespace, and MRI→MR aliasing.
for more information, see https://pre-commit.ci
DCO Remediation Commit for Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com> I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: 5394658 I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: 24665aa I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: 798f8af I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: b3a6ac2 I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: a446448 I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: d7f134c I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: ce7850f I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: 01b88fa I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: 821fc9a Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
…endrasinhdata7/MONAI into clinical-dicom-preprocessing
- Remove MetaTensor import/type hint to fix CI failures - Fix weak assertions with OR operator (now separate assertions) - Add verification of all key constructor arguments - Use Mock instead of lambda to avoid lint warnings - Update docstrings and error handling - Address all previous review feedback
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/tests/test_clinical_preprocessing.pymonai/transforms/clinical_preprocessing.py
🧬 Code graph analysis (1)
monai/tests/test_clinical_preprocessing.py (1)
monai/transforms/clinical_preprocessing.py (5)
get_ct_preprocessing_pipeline(20-39)get_mri_preprocessing_pipeline(42-61)preprocess_dicom_series(64-103)UnsupportedModalityError(15-17)ModalityTypeError(10-12)
🪛 Ruff (0.14.8)
monai/transforms/clinical_preprocessing.py
82-82: Avoid specifying long messages outside the exception class
(TRY003)
98-100: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-py3 (3.11)
Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
DCO Remediation Commit for Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com> I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: 5394658 I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: 24665aa I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: 798f8af I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: b3a6ac2 I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: a446448 I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: d7f134c I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: ce7850f I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: 01b88fa I, Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>, hereby add my Signed-off-by to this commit: 821fc9a Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
…endrasinhdata1/MONAI into clinical-dicom-preprocessing
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
monai/transforms/clinical_preprocessing.py (1)
76-98: Logic is correct - handles type validation, normalization, and pipeline selection properly.The string normalization (strip + uppercase) and MRI synonym mapping appropriately handle input variations.
Optional: Static analysis (Ruff TRY003) suggests defining exception messages in exception class constructors. Current approach provides better context and is acceptable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
monai/transforms/clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/transforms/clinical_preprocessing.py
🧬 Code graph analysis (1)
monai/transforms/clinical_preprocessing.py (5)
monai/data/meta_tensor.py (1)
MetaTensor(52-609)monai/transforms/compose.py (1)
Compose(123-393)monai/transforms/io/array.py (1)
LoadImage(109-305)monai/transforms/utility/array.py (1)
EnsureChannelFirst(175-234)monai/transforms/intensity/array.py (2)
ScaleIntensityRange(959-1013)NormalizeIntensity(816-925)
🪛 Ruff (0.14.8)
monai/transforms/clinical_preprocessing.py
77-77: Avoid specifying long messages outside the exception class
(TRY003)
93-95: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: build-docs
- GitHub Check: packaging
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-pytorch (2.6.0)
🔇 Additional comments (5)
monai/transforms/clinical_preprocessing.py (5)
1-8: LGTM - Imports and module structure correct.Module docstring is clear. MetaTensor import addresses past review feedback.
11-18: LGTM - Exception classes appropriately defined.Exception hierarchy (TypeError for type, ValueError for unsupported value) is correct.
21-40: LGTM - CT pipeline correctly implements standard soft tissue windowing.The HU window [-1000, 400] is a reasonable general default. Pipeline composition is correct.
43-62: LGTM - MRI pipeline correctly handles non-zero normalization.Using
nonzero=Trueappropriately excludes background zeros from normalization statistics.
102-108: LGTM - Public API exports complete.All five public symbols are correctly exported.
- Add clinical_preprocessing module with CT and MRI pipelines - Add ModalityTypeError and UnsupportedModalityError exceptions - Add get_ct_preprocessing_pipeline() for CT intensity normalization (-1000 to 400 HU) - Add get_mri_preprocessing_pipeline() for MRI nonzero normalization - Add preprocess_dicom_series() for modality-specific preprocessing - Add comprehensive unit tests covering all functionality - Update __init__.py exports for new module Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
monai/transforms/clinical_preprocessing.py (1)
29-103: Implementation is solid.The module structure, exception classes, pipeline factories, and modality routing are well-implemented. Docstrings meet guidelines, type hints are present, and validation logic is correct.
Static analysis flags error message placement (TRY003) on lines 90 and 99-101, suggesting exception class constructors instead of inline f-strings. Current approach is clear and includes context-specific values, so this is optional.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/__init__.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/transforms/__init__.pymonai/tests/test_clinical_preprocessing.pymonai/transforms/clinical_preprocessing.py
🧬 Code graph analysis (3)
monai/transforms/__init__.py (1)
monai/transforms/clinical_preprocessing.py (5)
ModalityTypeError(29-30)UnsupportedModalityError(33-34)get_ct_preprocessing_pipeline(37-56)get_mri_preprocessing_pipeline(59-72)preprocess_dicom_series(75-103)
monai/tests/test_clinical_preprocessing.py (1)
monai/transforms/clinical_preprocessing.py (5)
ModalityTypeError(29-30)UnsupportedModalityError(33-34)get_ct_preprocessing_pipeline(37-56)get_mri_preprocessing_pipeline(59-72)preprocess_dicom_series(75-103)
monai/transforms/clinical_preprocessing.py (5)
monai/transforms/compose.py (1)
Compose(123-393)monai/transforms/utility/array.py (1)
EnsureChannelFirst(175-234)monai/transforms/io/array.py (1)
LoadImage(109-305)monai/transforms/intensity/array.py (2)
NormalizeIntensity(816-925)ScaleIntensityRange(959-1013)monai/transforms/utils_pytorch_numpy_unification.py (2)
clip(94-101)nonzero(189-200)
🪛 Ruff (0.14.8)
monai/tests/test_clinical_preprocessing.py
105-105: Do not catch blind exception: Exception
(BLE001)
monai/transforms/clinical_preprocessing.py
90-90: Avoid specifying long messages outside the exception class
(TRY003)
99-101: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: flake8-py3 (mypy)
🔇 Additional comments (2)
monai/transforms/__init__.py (1)
684-691: LGTM.Re-exports follow existing patterns and correctly expose the new clinical preprocessing API.
monai/tests/test_clinical_preprocessing.py (1)
27-77: Pipeline and error tests are thorough.Tests validate pipeline structure, transform parameters, and exception handling with clear assertions.
- Fix test_modality_case_insensitivity to actually test function - Remove broad Exception catch in integration test - Improve error handling in tests Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
monai/transforms/clinical_preprocessing.py (3)
37-56: Consider enriching the docstring.The pipeline is correct, but the docstring could explain the clinical rationale for the HU window [-1000, 400], which covers lung and soft tissue intensities typical in CT imaging.
Enhanced docstring
def get_ct_preprocessing_pipeline() -> Compose: """ Create a preprocessing pipeline for CT images. Returns: - Compose: Transform composition for CT preprocessing. + Compose: Transform composition for CT preprocessing. + Applies Hounsfield Unit (HU) windowing [-1000, 400] scaled to [0, 1]. + This range captures lung (-1000 to -400 HU) and soft tissue (0 to 100 HU) contrast. """
59-72: Consider enriching the docstring.The pipeline is correct, but the docstring could explain that
nonzero=Trueexcludes background voxels from normalization statistics, which is appropriate for MRI where background is typically zero.Enhanced docstring
def get_mri_preprocessing_pipeline() -> Compose: """ Create a preprocessing pipeline for MRI images. Returns: - Compose: Transform composition for MRI preprocessing. + Compose: Transform composition for MRI preprocessing. + Normalizes intensities using nonzero voxels only, excluding background regions + typical in MRI acquisitions. """
75-75: Return type could be more specific.
LoadImage(image_only=True)returnsMetaTensor, notAny. UsingMetaTensorimproves type safety and IDE support.More precise type hint
+from monai.data import MetaTensor + -def preprocess_dicom_series(path: str, modality: str) -> Any: +def preprocess_dicom_series(path: str, modality: str) -> MetaTensor:Update the docstring:
Returns: - The preprocessed image data. + MetaTensor: The preprocessed image data with metadata.monai/tests/test_clinical_preprocessing.py (1)
79-95: Broad exception catch masks potential issues.Lines 93-95 catch all exceptions to avoid file I/O errors. While the intent is clear (test modality parsing only), this can hide unexpected failures. Consider mocking
LoadImagefor cleaner isolation.Mock-based alternative
+from unittest.mock import Mock, patch + +@patch("monai.transforms.clinical_preprocessing.LoadImage") -def test_modality_case_insensitivity(): +def test_modality_case_insensitivity(mock_load): """Test case-insensitive modality handling.""" + mock_load.return_value = Mock(return_value=Mock()) + for modality in ["CT", "ct", "Ct", "CT ", "MR", "mr", "MRI", "mri", " MrI "]: - try: - preprocess_dicom_series("non_existent_file.dcm", modality) - except (ModalityTypeError, UnsupportedModalityError): - pytest.fail(f"Modality {modality!r} should be accepted") - except FileNotFoundError: - pass - except Exception: - pass + # Should not raise modality errors + result = preprocess_dicom_series("dummy.dcm", modality) + assert result is not None
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/tests/test_clinical_preprocessing.pymonai/transforms/clinical_preprocessing.py
🧬 Code graph analysis (2)
monai/tests/test_clinical_preprocessing.py (1)
monai/transforms/clinical_preprocessing.py (5)
ModalityTypeError(29-30)UnsupportedModalityError(33-34)get_ct_preprocessing_pipeline(37-56)get_mri_preprocessing_pipeline(59-72)preprocess_dicom_series(75-103)
monai/transforms/clinical_preprocessing.py (4)
monai/transforms/compose.py (1)
Compose(123-393)monai/transforms/utility/array.py (1)
EnsureChannelFirst(175-234)monai/transforms/io/array.py (1)
LoadImage(109-305)monai/transforms/intensity/array.py (2)
NormalizeIntensity(816-925)ScaleIntensityRange(959-1013)
🪛 Ruff (0.14.8)
monai/tests/test_clinical_preprocessing.py
93-95: try-except-pass detected, consider logging the exception
(S110)
93-93: Do not catch blind exception: Exception
(BLE001)
monai/transforms/clinical_preprocessing.py
90-90: Avoid specifying long messages outside the exception class
(TRY003)
99-101: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (9)
monai/transforms/clinical_preprocessing.py (4)
1-27: Module structure is clean.Imports and module docstring are appropriate.
29-34: Exception classes are well-defined.Appropriate exception hierarchy and clear docstrings.
89-101: Error messages are informative.Ruff TRY003 suggests defining messages in the exception class, but the current approach includes context (original modality, supported values), which aids debugging. Current implementation is acceptable.
106-112: Exports are correct.All public symbols properly declared in
__all__.monai/tests/test_clinical_preprocessing.py (5)
27-43: Pipeline structure test is thorough.Validates all transform types and key parameters correctly.
46-56: Pipeline structure test is thorough.Validates all transform types and key parameters correctly.
59-64: Type validation test is correct.Properly uses
pytest.raisesand validates error message.
67-76: Unsupported modality test is correct.Properly validates exception type and message content.
98-110: Integration test is well-structured.Creates realistic test data and validates preprocessing output for both modalities.
- Add MetaTensor import and return type for better type safety - Improve docstrings with clinical rationale for CT and MRI pipelines - Fix test_modality_case_insensitivity using proper mocking - Remove broad exception catching in tests Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
Hitendrasinhdata7
force-pushed
the
clinical-dicom-preprocessing
branch
from
2d5e2d3 to
b3e8f87
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
monai/transforms/clinical_preprocessing.py (3)
75-75: Return type hint is too vague.
Anyprovides no type safety. The function returns the output of aComposepipeline, which withLoadImage(image_only=True)produces aMetaTensor. UseMetaTensoror at minimum a union of possible tensor types.Proposed fix
Add the import:
+from monai.data import MetaTensor from monai.transforms import ( Compose, EnsureChannelFirst, LoadImage, NormalizeIntensity, ScaleIntensityRange, )Update the signature and docstring:
-def preprocess_dicom_series(path: str, modality: str) -> Any: +def preprocess_dicom_series(path: str, modality: str) -> MetaTensor: """Preprocess a DICOM series or file based on imaging modality. Args: path: Path to the DICOM file or directory containing a DICOM series. modality: Imaging modality. Supported values are "CT", "MR", and "MRI" (case-insensitive). Returns: - The preprocessed image data. + MetaTensor: Preprocessed image tensor with metadata. Raises: ModalityTypeError: If modality is not a string. UnsupportedModalityError: If the provided modality is not supported. """As per coding guidelines, return values must be documented with specific types in Google-style docstrings.
37-56: CT pipeline docstring could explain clinical rationale.The HU window [-1000, 400] is standard for soft tissue and lung, but the docstring doesn't explain why these values were chosen.
Enhanced docstring
def get_ct_preprocessing_pipeline() -> Compose: """ Create a preprocessing pipeline for CT images. Returns: - Compose: Transform composition for CT preprocessing. + Compose: Transform composition for CT preprocessing. Applies HU windowing + [-1000, 400] scaled to [0, 1] with clipping, suitable for soft tissue + and lung visualization. """
59-72: MRI pipeline docstring could explain normalization approach.The
nonzero=Trueparameter excludes background zeros from statistics, which is important for MRI but not obvious from the brief docstring.Enhanced docstring
def get_mri_preprocessing_pipeline() -> Compose: """ Create a preprocessing pipeline for MRI images. Returns: - Compose: Transform composition for MRI preprocessing. + Compose: Transform composition for MRI preprocessing. Normalizes using + mean/std computed over non-zero voxels only, appropriate for MRI + data with background regions. """monai/tests/test_clinical_preprocessing.py (1)
80-90: Consider testing "MR" modality explicitly.Test covers "MRI" but not the distinct "MR" synonym. The integration test uses only "CT" and "MRI", leaving "MR" untested in actual execution.
Add MR test case
@patch("monai.transforms.clinical_preprocessing.LoadImage") def test_mr_modality_distinct(mock_load): """Test MR modality is handled separately from MRI.""" mock_load.return_value = Mock(return_value=Mock()) result = preprocess_dicom_series("dummy.dcm", "MR") assert result is not NoneOr add "MR" to the integration test loop at line 101.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/tests/test_clinical_preprocessing.pymonai/transforms/clinical_preprocessing.py
🧬 Code graph analysis (1)
monai/tests/test_clinical_preprocessing.py (3)
monai/transforms/utility/array.py (1)
EnsureChannelFirst(175-234)monai/transforms/intensity/array.py (2)
NormalizeIntensity(816-925)ScaleIntensityRange(959-1013)monai/transforms/clinical_preprocessing.py (5)
ModalityTypeError(29-30)UnsupportedModalityError(33-34)get_ct_preprocessing_pipeline(37-56)get_mri_preprocessing_pipeline(59-72)preprocess_dicom_series(75-103)
🪛 Ruff (0.14.8)
monai/transforms/clinical_preprocessing.py
90-90: Avoid specifying long messages outside the exception class
(TRY003)
99-101: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (10)
monai/transforms/clinical_preprocessing.py (4)
1-27: LGTM: Imports and module structure.Clean imports and clear module docstring.
29-35: LGTM: Exception classes.Well-defined custom exceptions with appropriate inheritance.
89-103: LGTM: Input validation and pipeline selection logic.Proper type checking, case normalization, and synonym handling. Exception messages are informative despite static analysis warnings.
106-112: LGTM: Public API exports.All public symbols correctly listed in
__all__.monai/tests/test_clinical_preprocessing.py (6)
28-45: LGTM: Comprehensive CT pipeline structure test.Validates all transforms, parameters, and the HU window configuration.
47-58: LGTM: Comprehensive MRI pipeline structure test.Validates all transforms and the
nonzero=Trueparameter for NormalizeIntensity.
60-66: LGTM: Type validation test.Correctly validates ModalityTypeError with pytest.raises and checks error message.
68-78: LGTM: Unsupported modality test.Validates UnsupportedModalityError and checks all expected strings in error message.
92-104: LGTM: Solid integration test.Uses real file I/O with temporary directory and validates results for both CT and MRI modalities.
1-26: LGTM: Clean test imports and structure.All necessary imports present and organized.
- Improve CT docstring with clinical rationale for HU windowing [-1000, 400] - Improve MRI docstring explaining nonzero=True for background exclusion - Add explicit test_mr_modality_distinct() for MR modality testing - Update integration test to cover CT, MR, and MRI modalities - Update return type annotation and docstring for MetaTensor Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
Hitendrasinhdata7
force-pushed
the
clinical-dicom-preprocessing
branch
from
85720ca to
5a4fbb5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
monai/transforms/clinical_preprocessing.py (1)
78-106: LGTM! Function is well-implemented with proper validation and error handling.Type hints, docstring, and logic are all correct. Input validation is thorough: type check for modality, normalization with strip/upper, and clear routing to appropriate pipelines.
The Ruff TRY003 warnings (lines 93, 102-104) about long exception messages are overly pedantic. Your f-string error messages include dynamic context (the invalid modality value), which is more helpful for debugging than static messages in the exception class.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/transforms/clinical_preprocessing.pymonai/tests/test_clinical_preprocessing.py
🧬 Code graph analysis (1)
monai/tests/test_clinical_preprocessing.py (1)
monai/transforms/clinical_preprocessing.py (5)
ModalityTypeError(28-29)UnsupportedModalityError(32-33)get_ct_preprocessing_pipeline(36-57)get_mri_preprocessing_pipeline(60-75)preprocess_dicom_series(78-106)
🪛 Ruff (0.14.8)
monai/transforms/clinical_preprocessing.py
93-93: Avoid specifying long messages outside the exception class
(TRY003)
102-104: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (10)
monai/transforms/clinical_preprocessing.py (4)
28-34: LGTM! Well-designed exception classes.Proper exception hierarchy and clear docstrings. ModalityTypeError inherits from TypeError for type validation failures, and UnsupportedModalityError from ValueError for invalid modality values.
36-57: LGTM! CT pipeline correctly implemented.HU windowing [-1000, 400] → [0, 1] with clipping is appropriate for soft tissue and lung visualization. Docstring clearly documents the transforms and rationale.
60-75: LGTM! MRI pipeline correctly implemented.NormalizeIntensity with nonzero=True appropriately handles MRI data with background regions by computing statistics over non-zero voxels only.
109-115: LGTM! Complete public API export.All public symbols correctly exported.
monai/tests/test_clinical_preprocessing.py (6)
12-25: LGTM! All necessary imports present.Clean import structure with all symbols used in the tests.
28-44: LGTM! Thorough pipeline structure validation.Tests validate all three transforms and their critical parameters, including the HU windowing values for CT preprocessing.
47-57: LGTM! Proper MRI pipeline validation.Validates transform composition and the critical nonzero=True parameter for MRI normalization.
60-77: LGTM! Proper exception testing.Both tests correctly use pytest.raises and validate error messages contain expected information. Good coverage of both custom exception types.
80-97: LGTM! Case insensitivity properly tested.Mocking strategy correctly avoids file I/O while testing modality normalization. The test suite validates various case and whitespace inputs, ensuring robust input handling.
100-112: LGTM! Solid integration test.End-to-end test with actual file creation and all three modality variants (CT, MR, MRI). Using NIfTI instead of DICOM is acceptable since LoadImage handles multiple formats, and this avoids test complexity.
Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
Hitendrasinhdata7
force-pushed
the
clinical-dicom-preprocessing
branch
from
9daef2c to
c273cf7
Compare
for more information, see https://pre-commit.ci
Hitendrasinhdata7
changed the title
-s - [x] Unit tests added - [x] __init__.py exports updated - [x] MONAI coding style followed ## Motivation Standardize preprocessing of clinical DICOM images for medical AI pipelines. ## Detailed Changes - Added clinical_preprocessing.py with CT/MRI pipelines - Added preprocess_dicom_series() function - Added custom exceptions - Added comprehensive unit tests - Updated __init__.py exports
- Add proper copyright headers - Fix type hints and imports - Follow MONAI coding standards - Improve test coverage Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
monai/transforms/clinical_preprocessing.py (2)
90-90: Return type annotation may be inaccurate.
LoadImage(image_only=True)returnsMetaTensorortorch.Tensor(when meta tracking is disabled), notnp.ndarray. The pipeline preserves torch tensor types. Consider updating the return type annotation totorch.Tensor(orMetaTensorif you want to be more specific, though that requires importing frommonai.data).Suggested type annotation
-def preprocess_medical_image(path: str, modality: str) -> Union[torch.Tensor, np.ndarray]: +def preprocess_medical_image(path: str, modality: str) -> torch.Tensor:And update the Returns section:
Returns: - Preprocessed image data as a tensor or numpy array. + Preprocessed image data as a torch.Tensor (or MetaTensor with metadata).Also applies to: 100-100
107-107: Optional: Consider exception class with default messages.Ruff flags TRY003 for specifying long messages outside the exception class. You could define exception classes with default messages, but the current implementation is clear and informative. This is low priority.
Alternative approach (if you prefer)
class ModalityTypeError(TypeError): - """Raised when modality is not a string.""" + """Raised when modality is not a string.""" + def __init__(self): + super().__init__("modality must be a string") class UnsupportedModalityError(ValueError): - """Raised when an unsupported modality is requested.""" + """Raised when an unsupported modality is requested.""" + def __init__(self, modality: str): + super().__init__( + f"Unsupported modality '{modality}'. Supported modalities: CT, MR, MRI" + )Then update usage:
- raise ModalityTypeError("modality must be a string") + raise ModalityTypeError() - raise UnsupportedModalityError( - f"Unsupported modality '{modality}'. Supported modalities: CT, MR, MRI" - ) + raise UnsupportedModalityError(modality)Also applies to: 116-118
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- monai/tests/test_clinical_preprocessing.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/transforms/clinical_preprocessing.py
🧬 Code graph analysis (1)
monai/transforms/clinical_preprocessing.py (4)
monai/transforms/compose.py (1)
Compose(123-393)monai/transforms/utility/array.py (1)
EnsureChannelFirst(175-234)monai/transforms/io/array.py (1)
LoadImage(109-305)monai/transforms/intensity/array.py (2)
NormalizeIntensity(816-925)ScaleIntensityRange(959-1013)
🪛 Ruff (0.14.8)
monai/transforms/clinical_preprocessing.py
107-107: Avoid specifying long messages outside the exception class
(TRY003)
116-118: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: flake8-py3 (mypy)
🔇 Additional comments (6)
monai/transforms/clinical_preprocessing.py (6)
1-29: LGTM on copyright, module docstring, and imports.Clean structure. Imports are well-organized.
32-37: Exception classes are well-defined.Appropriate inheritance and clear docstrings.
40-65: CT pipeline is properly configured.HU window [-1000, 400] appropriately captures lung and soft tissue contrast. Documentation is thorough.
68-87: MRI pipeline correctly uses nonzero normalization.Appropriate for excluding background regions in MRI data. Documentation is clear.
123-142: Clean backward compatibility wrapper.Good practice to maintain the legacy function name while delegating to the more accurately named function. Documentation clarifies support for formats beyond DICOM.
Note: The return type annotation issue mentioned for
preprocess_medical_imagealso applies here.
145-152: Comprehensive export list.All public symbols are properly exported via
__all__.
- Change return type from Union[Tensor, np.ndarray] to torch.Tensor - Refactor exception classes with default messages (fixes TRY003) - Update tests to match new exception format Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
- Merge remote updates into local branch - Fix return type from Union[Tensor, np.ndarray] to torch.Tensor - Refactor exception classes with default messages (fix TRY003) - Update tests to match new exception format Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
monai/tests/test_clinical_preprocessing.py (2)
102-115: Optional: Simplify test_cases to a list of strings.The boolean in each tuple is unused (discarded with
_on line 115). Since all cases expect success, consider simplifying to a plain list.🔎 Suggested simplification
- test_cases = [ - ("CT", True), - ("ct", True), - ("Ct", True), - ("CT ", True), - (" CT", True), - ("MR", True), - ("mr", True), - ("MRI", True), - ("mri", True), - (" MrI ", True), - ] + test_cases = ["CT", "ct", "Ct", "CT ", " CT", "MR", "mr", "MRI", "mri", " MrI "] - for modality, _ in test_cases: + for modality in test_cases: result = preprocess_dicom_series("dummy.dcm", modality) assert result is not None, f"Failed for modality: '{modality}'" result2 = preprocess_medical_image("dummy.dcm", modality) assert result2 is not None, f"preprocess_medical_image failed for modality: '{modality}'"
148-161: Optional: Add output property validations.The integration test validates existence and shape attribute but could verify output characteristics like dimensionality, value ranges, or tensor type to strengthen the test.
Example enhancements
for modality in ["CT", "MR", "MRI"]: result = preprocess_dicom_series(str(test_file), modality) assert result is not None assert hasattr(result, "shape") assert len(result.shape) == 4 # (C, H, W, D) assert result.shape[0] == 1 # single channel if modality == "CT": # CT output should be in [0, 1] due to ScaleIntensityRange assert result.min() >= 0.0 assert result.max() <= 1.0
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.py(1 hunks)monai/transforms/clinical_preprocessing.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- monai/transforms/clinical_preprocessing.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/tests/test_clinical_preprocessing.py
🧬 Code graph analysis (1)
monai/tests/test_clinical_preprocessing.py (1)
monai/transforms/clinical_preprocessing.py (6)
ModalityTypeError(29-32)UnsupportedModalityError(35-40)get_ct_preprocessing_pipeline(43-68)get_mri_preprocessing_pipeline(71-90)preprocess_dicom_series(125-143)preprocess_medical_image(93-121)
🔇 Additional comments (1)
monai/tests/test_clinical_preprocessing.py (1)
1-161: Comprehensive test coverage with solid structure.The test module thoroughly covers pipeline composition, error handling (type and modality errors), case-insensitive handling, edge cases, and integration with real files. Both
preprocess_dicom_seriesandpreprocess_medical_imageare consistently tested. All past review feedback has been addressed.
- Simplify test_cases to plain list (remove unused boolean) - Strengthen integration test with shape and value validations - Verify CT output range [0, 1] and single channel output Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
monai/tests/test_clinical_preprocessing.py (3)
97-108: Simplify nested Mock construction.Line 100:
Mock(return_value=Mock())works but is unclear. Create a single mock object for cleaner setup.🔎 Cleaner mock setup
def test_modality_case_insensitivity(mock_load): """Test case-insensitive modality handling with whitespace trimming.""" - mock_load.return_value = Mock(return_value=Mock()) + mock_image = Mock() + mock_load.return_value = Mock(return_value=mock_image) test_cases = ["CT", "ct", "Ct", "CT ", " CT", "MR", "mr", "MRI", "mri", " MrI "]
111-134: LGTM: Edge cases and MR/MRI distinction validated.Lines 114, 124 use same nested Mock pattern as previous test. Consider applying the cleaner mock setup from the earlier suggestion consistently.
137-160: LGTM: Comprehensive integration test with actual I/O.CT scaling to [0, 1] validated (lines 151-154). MR/MRI results not validated beyond shape, but pipeline structure tests cover transformation parameters.
Optional: Add MR/MRI normalization validation
Consider adding statistical validation for MR/MRI results to confirm NormalizeIntensity effect:
if modality in ["MR", "MRI"]: # NormalizeIntensity should yield mean≈0, std≈1 for nonzero voxels nonzero_mask = result != 0 if nonzero_mask.any(): assert abs(result[nonzero_mask].mean()) < 0.1 assert abs(result[nonzero_mask].std() - 1.0) < 0.2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
monai/tests/test_clinical_preprocessing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/tests/test_clinical_preprocessing.py
🔇 Additional comments (6)
monai/tests/test_clinical_preprocessing.py (6)
12-26: LGTM: Imports properly organized.Follows PEP8 ordering (standard library, third-party, local) and all symbols are used.
29-45: LGTM: Comprehensive CT pipeline validation.Assertions cover transform types, count, and all ScaleIntensityRange parameters. Line 37 uses identity check (
__class__ is) rather thanisinstance()- acceptable for strict type matching.
48-58: LGTM: MRI pipeline validation matches CT test pattern.All transform types and key parameters validated.
61-71: LGTM: Exception handling verified correctly.Tests both functions with invalid types, uses
pytest.raises, validates error messages.
74-94: LGTM: Unsupported modality handling validated thoroughly.Error messages checked for all supported modalities in addition to the unsupported one.
1-160: Test coverage is comprehensive.File validates pipeline structure, error handling, case sensitivity, edge cases, and end-to-end integration. Docstrings are brief but standard for test functions.
- Clean up nested Mock construction with explicit mock_image - Add statistical validation for MRI normalization (mean≈0, std≈1) - Maintain consistent mock patterns across all test functions Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
- Accept auto-formatting from /black - Apply test improvements: cleaner mocks and MRI validation Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
ericspod
changed the title
-s - [x] Unit tests added - [x] __init__.py exports updated - [x] MONAI coding style followed ## Motivation Standardize preprocessing of clinical DICOM images for medical AI pipelines. ## Detailed Changes - Added clinical_preprocessing.py with CT/MRI pipelines - Added preprocess_dicom_series() function - Added custom exceptions - Added comprehensive unit tests - Updated __init__.py exports1 participant
Description Add clinical preprocessing utilities for CT and MRI modalities.
Status Ready for review
Types of changes
PR Checklist
-s__init__.pyexports updatedMotivation
Standardize preprocessing of clinical DICOM images for medical AI pipelines.
Detailed Changes
clinical_preprocessing.pywith CT/MRI pipelinespreprocess_dicom_series()function__init__.pyexportsChanges
get_ct_preprocessing_pipeline()andget_mri_preprocessing_pipeline()with Google-style docstrings.preprocess_dicom_series()to include type hints and more informative exception messages.tests/test_clinical_preprocessing.pycovering:Motivation
These utilities help standardize preprocessing of clinical DICOM images and facilitate AI model development in medical imaging.
Tests
All tests have been added and verified locally using
pytest.