Add clinical DICOM preprocessing files, workflow PDF, and example notebooks #8658
Conversation
Hitendrasinhdata7
commented
Dec 14, 2025
Hitendrasinhdata7
commented
- Adds clinical preprocessing transforms for CT and MRI
- Includes workflow documentation (PDF)
- Includes example notebooks for usage
- Includes unit tests
…nd example notebooks
Hitendrasinhdata7
requested review from
KumoLiu,
Nic-Ma and
ericspod
as code owners
WalkthroughA new preprocessing module introduces clinical imaging pipelines for CT and MRI data. The module exposes three functions: Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
for more information, see https://pre-commit.ci
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
monai/transforms/clinical_preprocessing.ipynb (1)
90-102: Consider parameterizing CT window + whether to preserve metadata (image_only).
Hardcodinga_min/a_max(Line 94) and always usingimage_only=True(Line 92/99) makes this less reusable for real clinical pipelines where windowing varies and downstream often needs spacing/orientation.
📜 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 (1)
monai/docs/clinical_dicom_workflow.pdfis excluded by!**/*.pdf
📒 Files selected for processing (2)
monai/tests/test_clinical_preprocessing.ipynb(1 hunks)monai/transforms/clinical_preprocessing.ipynb(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.8)
monai/transforms/clinical_preprocessing.ipynb
43-43: 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: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: packaging
- GitHub Check: build-docs
- GitHub Check: flake8-py3 (mypy)
🔇 Additional comments (1)
monai/transforms/clinical_preprocessing.ipynb (1)
61-67: Verify DICOM series loading behavior forLoadImagein this workflow.
Depending on whetherdicom_pathis a directory,LoadImagemay require an explicit reader/config for DICOM series; please confirm this works against a real series (CT + MR) in your intended environment.Also applies to: 90-102
| "def test_ct_windowing():\n", | ||
| " \"\"\"\n", | ||
| " Test the CTWindowingTransform to ensure output is scaled between 0 and 1\n", | ||
| " and the shape is preserved.\n", | ||
| " \"\"\"\n", | ||
| " # Mock CT image with Hounsfield Units\n", | ||
| " sample_ct = np.random.randint(-1024, 2048, size=(64, 64, 64), dtype=np.int16)\n", | ||
| "\n", | ||
| " transform = CTWindowingTransform()\n", | ||
| " output = transform(sample_ct)\n", | ||
| "\n", | ||
| " # Output must be in [0,1]\n", | ||
| " assert output.min() >= 0.0\n", | ||
| " assert output.max() <= 1.0\n", | ||
| " # Shape should be preserved\n", | ||
| " assert output.shape == sample_ct.shape\n", | ||
| "\n", |
There was a problem hiding this comment.
Assertions may break depending on transform return type (numpy vs torch/MetaTensor) + RNG flakiness.
- Line 25/42: seed RNG for determinism.
- Line 31-32: guard float comparisons with tolerance, and normalize
outputto a numpy array beforemin/max/mean/std. - Line 52: require both mean≈0 and std≈1 (optionally on nonzero mask), not
or.
def test_ct_windowing():
+ rng = np.random.default_rng(0)
# Mock CT image with Hounsfield Units
- sample_ct = np.random.randint(-1024, 2048, size=(64, 64, 64), dtype=np.int16)
+ sample_ct = rng.integers(-1024, 2048, size=(64, 64, 64), dtype=np.int16)
transform = CTWindowingTransform()
- output = transform(sample_ct)
+ output = transform(sample_ct)
+ output = np.asarray(output)
# Output must be in [0,1]
- assert output.min() >= 0.0
- assert output.max() <= 1.0
+ assert np.isfinite(output).all()
+ assert output.min() >= -1e-6
+ assert output.max() <= 1.0 + 1e-6
# Shape should be preserved
assert output.shape == sample_ct.shape
def test_mri_normalization():
+ rng = np.random.default_rng(0)
# Mock MRI image with random float values
- sample_mri = np.random.rand(64, 64, 64)
+ sample_mri = rng.random((64, 64, 64), dtype=np.float32)
transform = MRINormalizationTransform()
- output = transform(sample_mri)
+ output = transform(sample_mri)
+ output = np.asarray(output)
# Shape should be preserved
assert output.shape == sample_mri.shape
# Values should be roughly normalized (mean near 0, std near 1)
- mean_val = np.mean(output)
- std_val = np.std(output)
- assert np.isclose(mean_val, 0, atol=0.1) or np.isclose(std_val, 1, atol=0.1)
+ mean_val = float(np.mean(output))
+ std_val = float(np.std(output))
+ assert np.isclose(mean_val, 0.0, atol=0.1)
+ assert np.isclose(std_val, 1.0, atol=0.1)Also applies to: 36-52
🤖 Prompt for AI Agents
In monai/tests/test_clinical_preprocessing.ipynb around lines 19 to 35 (and
likewise apply the same fix to lines 36 to 52), the test is flaky because the
RNG isn't seeded, comparisons assume a specific array type, and float
comparisons use strict equality; to fix: seed the RNG at the start of each test
for determinism, convert the transform output to a numpy array (or unwrap
MetaTensor/torch tensor) before calling min/max/mean/std, use tolerant
comparisons (e.g. abs(a - b) <= tol) for float checks, and for the normalization
test require both mean ≈ 0 AND std ≈ 1 (optionally computed over a nonzero mask)
rather than using OR.
| "def preprocess_dicom_series(dicom_path, modality):\n", | ||
| " modality = modality.upper()\n", | ||
| " if modality == 'CT':\n", | ||
| " transform = get_ct_preprocessing_pipeline()\n", | ||
| " elif modality == 'MRI':\n", | ||
| " transform = get_mri_preprocessing_pipeline()\n", | ||
| " else:\n", | ||
| " raise ValueError(\"Unsupported modality. Use 'CT' or 'MRI'.\")\n", | ||
| " image = transform(dicom_path)\n", | ||
| " return image" | ||
| ] |
There was a problem hiding this comment.
Modality handling is too strict (DICOM uses MR) and input validation is missing.
- Line 125: guard non-string modality.
- Line 128-131: accept
MRas synonym for MRI (and trim whitespace). - Line 131: Ruff TRY003—keep message short or factor to a constant.
def preprocess_dicom_series(dicom_path, modality):
- modality = modality.upper()
- if modality == 'CT':
+ if not isinstance(modality, str):
+ raise TypeError("modality must be a string")
+ modality = modality.strip().upper()
+ if modality == "CT":
transform = get_ct_preprocessing_pipeline()
- elif modality == 'MRI':
+ elif modality in ("MR", "MRI"):
transform = get_mri_preprocessing_pipeline()
else:
- raise ValueError("Unsupported modality. Use 'CT' or 'MRI'.")
+ raise ValueError("Unsupported modality")
image = transform(dicom_path)
return image🤖 Prompt for AI Agents
In monai/transforms/clinical_preprocessing.ipynb around lines 124 to 134, the
modality handling is too strict and lacks input validation; update the function
to (1) validate modality is a string and raise a short constant error message if
not, (2) trim whitespace and uppercase the modality, treating "MR" as a synonym
for "MRI" before branching to select the CT or MRI pipeline, and (3) factor the
unsupported-modality message into a module-level constant (or keep it very
short) to satisfy Ruff TRY003; implement these changes so transform selection
accepts "MR" and non-string inputs are rejected cleanly.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
monai/transforms/clinical_preprocessing.py (1)
58-59: Bare exception message.Static analysis flags this as a bare string in exception. While functional, it's better practice to define specific error messages as constants or be more descriptive.
Consider making the error message more informative:
if not isinstance(modality, str): - raise TypeError("modality must be a string") + raise TypeError(f"modality must be a string, got {type(modality).__name__}")
📜 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 (1)
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 (1)
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 (1)
NormalizeIntensity(816-925)
🪛 Ruff (0.14.8)
monai/transforms/clinical_preprocessing.py
59-59: Avoid specifying long messages outside the exception class
(TRY003)
68-68: 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). (18)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: packaging
🔇 Additional comments (1)
monai/transforms/clinical_preprocessing.py (1)
44-46: Type hint should includePathLiketo align withLoadImage's accepted types.
Union[str, bytes]is too restrictive. The function passesdicom_pathtoLoadImage, which acceptsSequence[PathLike] | PathLike. Update toUnion[str, bytes, PathLike](or similar) to match the docstring "Path to DICOM file or directory" and actual downstream usage.[suggested_refactor_recommended]
| import numpy as np | ||
|
|
||
| from monai.transforms import ScaleIntensityRange, NormalizeIntensity |
There was a problem hiding this comment.
Missing import of the module under test.
The test file doesn't import monai.transforms.clinical_preprocessing, which contains the functions this test module should verify (get_ct_preprocessing_pipeline(), get_mri_preprocessing_pipeline(), preprocess_dicom_series()).
Add the import:
import numpy as np
from monai.transforms import ScaleIntensityRange, NormalizeIntensity
+from monai.transforms.clinical_preprocessing import (
+ get_ct_preprocessing_pipeline,
+ get_mri_preprocessing_pipeline,
+ preprocess_dicom_series,
+)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import numpy as np | |
| from monai.transforms import ScaleIntensityRange, NormalizeIntensity | |
| import numpy as np | |
| from monai.transforms import ScaleIntensityRange, NormalizeIntensity | |
| from monai.transforms.clinical_preprocessing import ( | |
| get_ct_preprocessing_pipeline, | |
| get_mri_preprocessing_pipeline, | |
| preprocess_dicom_series, | |
| ) |
🤖 Prompt for AI Agents
In monai/tests/test_clinical_preprocessing.py around lines 1 to 3, the test file
fails to import the module under test (monai.transforms.clinical_preprocessing)
so the functions get_ct_preprocessing_pipeline, get_mri_preprocessing_pipeline,
and preprocess_dicom_series are unavailable; add an import for
monai.transforms.clinical_preprocessing (or specific functions) at the top of
the file so the tests can reference those functions.
| def test_ct_windowing_range_and_shape(): | ||
| rng = np.random.default_rng(0) | ||
|
|
||
| sample_ct = rng.integers( | ||
| -1024, 2048, size=(64, 64, 64), dtype=np.int16 | ||
| ) | ||
|
|
||
| transform = ScaleIntensityRange( | ||
| a_min=-1000, | ||
| a_max=400, | ||
| b_min=0.0, | ||
| b_max=1.0, | ||
| clip=True, | ||
| ) | ||
|
|
||
| output = transform(sample_ct) | ||
| output = np.asarray(output) | ||
|
|
||
| assert output.shape == sample_ct.shape | ||
| assert np.isfinite(output).all() | ||
| assert output.min() >= -1e-6 | ||
| assert output.max() <= 1.0 + 1e-6 | ||
|
|
There was a problem hiding this comment.
Test doesn't validate the pipeline function.
This test validates ScaleIntensityRange directly but doesn't test get_ct_preprocessing_pipeline(). Per coding guidelines, new definitions require test coverage.
Consider testing the actual pipeline function. Note that get_ct_preprocessing_pipeline() includes LoadImage which requires file I/O, so you'll need to either mock the file loading or create temporary test files:
def test_ct_windowing_range_and_shape():
rng = np.random.default_rng(0)
sample_ct = rng.integers(-1024, 2048, size=(64, 64, 64), dtype=np.int16)
# Direct transform test (current approach)
transform = ScaleIntensityRange(
a_min=-1000, a_max=400, b_min=0.0, b_max=1.0, clip=True
)
output = transform(sample_ct)
output = np.asarray(output)
assert output.shape == sample_ct.shape
assert np.isfinite(output).all()
assert output.min() >= -1e-6
assert output.max() <= 1.0 + 1e-6
def test_ct_preprocessing_pipeline():
"""Test get_ct_preprocessing_pipeline with mocked file loading."""
# Add test for the actual pipeline function
pass| def test_mri_normalization_mean_std(): | ||
| rng = np.random.default_rng(0) | ||
|
|
||
| sample_mri = rng.random((64, 64, 64), dtype=np.float32) | ||
|
|
||
| transform = NormalizeIntensity(nonzero=True) | ||
|
|
||
| output = transform(sample_mri) | ||
| output = np.asarray(output) | ||
|
|
||
| mean_val = float(output.mean()) | ||
| std_val = float(output.std()) | ||
|
|
||
| assert output.shape == sample_mri.shape | ||
| assert np.isclose(mean_val, 0.0, atol=0.1) | ||
| assert np.isclose(std_val, 1.0, atol=0.1) |
There was a problem hiding this comment.
Test doesn't validate the pipeline function.
Same issue as above—this validates NormalizeIntensity but not get_mri_preprocessing_pipeline() or preprocess_dicom_series().
| def get_ct_preprocessing_pipeline(): | ||
| """ | ||
| CT preprocessing pipeline using standard HU windowing. | ||
| """ | ||
| return Compose( | ||
| [ | ||
| LoadImage(image_only=True), | ||
| EnsureChannelFirst(), | ||
| ScaleIntensityRange( | ||
| a_min=-1000, | ||
| a_max=400, | ||
| b_min=0.0, | ||
| b_max=1.0, | ||
| clip=True, | ||
| ), | ||
| ] | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Incomplete docstring.
Per coding guidelines, docstrings must follow Google style and document return values. This function returns a Compose object but the docstring doesn't specify it.
Based on coding guidelines, complete the docstring:
def get_ct_preprocessing_pipeline():
"""
- CT preprocessing pipeline using standard HU windowing.
+ Build a CT preprocessing pipeline using standard HU windowing.
+
+ The pipeline applies LoadImage, EnsureChannelFirst, and ScaleIntensityRange
+ with HU window [-1000, 400] normalized to [0.0, 1.0].
+
+ Returns:
+ Compose: A composed transform pipeline for CT preprocessing.
"""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_ct_preprocessing_pipeline(): | |
| """ | |
| CT preprocessing pipeline using standard HU windowing. | |
| """ | |
| return Compose( | |
| [ | |
| LoadImage(image_only=True), | |
| EnsureChannelFirst(), | |
| ScaleIntensityRange( | |
| a_min=-1000, | |
| a_max=400, | |
| b_min=0.0, | |
| b_max=1.0, | |
| clip=True, | |
| ), | |
| ] | |
| ) | |
| def get_ct_preprocessing_pipeline(): | |
| """ | |
| Build a CT preprocessing pipeline using standard HU windowing. | |
| The pipeline applies LoadImage, EnsureChannelFirst, and ScaleIntensityRange | |
| with HU window [-1000, 400] normalized to [0.0, 1.0]. | |
| Returns: | |
| Compose: A composed transform pipeline for CT preprocessing. | |
| """ | |
| return Compose( | |
| [ | |
| LoadImage(image_only=True), | |
| EnsureChannelFirst(), | |
| ScaleIntensityRange( | |
| a_min=-1000, | |
| a_max=400, | |
| b_min=0.0, | |
| b_max=1.0, | |
| clip=True, | |
| ), | |
| ] | |
| ) |
🤖 Prompt for AI Agents
In monai/transforms/clinical_preprocessing.py around lines 12 to 28, the
function docstring is incomplete and must follow Google style; update the
docstring to include a one-line summary, a brief description of behavior (HU
windowing for CT), and a Returns section that states it returns a
monai.transforms.Compose object (describe that it contains LoadImage,
EnsureChannelFirst, and ScaleIntensityRange configured for HU -1000 to 400
mapped to 0.0-1.0 with clipping). Keep it concise and use Google-style sections
(Args: none or omit if no args, Returns: monai.transforms.Compose: description).
| def get_mri_preprocessing_pipeline(): | ||
| """ | ||
| MRI preprocessing pipeline using intensity normalization. | ||
| """ | ||
| return Compose( | ||
| [ | ||
| LoadImage(image_only=True), | ||
| EnsureChannelFirst(), | ||
| NormalizeIntensity(nonzero=True), | ||
| ] | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Incomplete docstring.
Same issue—missing Returns section per Google-style docstring requirements.
def get_mri_preprocessing_pipeline():
"""
- MRI preprocessing pipeline using intensity normalization.
+ Build an MRI preprocessing pipeline using intensity normalization.
+
+ The pipeline applies LoadImage, EnsureChannelFirst, and NormalizeIntensity
+ with nonzero=True to normalize only non-zero voxels.
+
+ Returns:
+ Compose: A composed transform pipeline for MRI preprocessing.
"""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_mri_preprocessing_pipeline(): | |
| """ | |
| MRI preprocessing pipeline using intensity normalization. | |
| """ | |
| return Compose( | |
| [ | |
| LoadImage(image_only=True), | |
| EnsureChannelFirst(), | |
| NormalizeIntensity(nonzero=True), | |
| ] | |
| ) | |
| def get_mri_preprocessing_pipeline(): | |
| """ | |
| Build an MRI preprocessing pipeline using intensity normalization. | |
| The pipeline applies LoadImage, EnsureChannelFirst, and NormalizeIntensity | |
| with nonzero=True to normalize only non-zero voxels. | |
| Returns: | |
| Compose: A composed transform pipeline for MRI preprocessing. | |
| """ | |
| return Compose( | |
| [ | |
| LoadImage(image_only=True), | |
| EnsureChannelFirst(), | |
| NormalizeIntensity(nonzero=True), | |
| ] | |
| ) |
🤖 Prompt for AI Agents
In monai/transforms/clinical_preprocessing.py around lines 31 to 41, the
function docstring for get_mri_preprocessing_pipeline is missing a Google-style
"Returns" section; update the docstring to include a Returns header that states
the function returns a monai.transforms.Compose object (or typing.Union[Compose,
Sequence[Transform]] if appropriate) describing the preprocessing pipeline
(loaded image with channel-first and normalized intensities), and ensure the
overall docstring follows Google style (short description, optional Args if any,
and the Returns section).
| def preprocess_dicom_series( | ||
| dicom_path: Union[str, bytes], | ||
| modality: str, | ||
| ): | ||
| """ | ||
| Preprocess a DICOM series based on modality. | ||
|
|
||
| Args: | ||
| dicom_path: Path to DICOM file or directory. | ||
| modality: CT, MR, or MRI. | ||
|
|
||
| Returns: | ||
| Preprocessed image. | ||
| """ | ||
| if not isinstance(modality, str): | ||
| raise TypeError("modality must be a string") | ||
|
|
||
| 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("Unsupported modality") | ||
|
|
||
| return transform(dicom_path) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Incomplete docstring and missing return type annotation.
The docstring lacks a Raises section and return type details. The function signature is missing a return type annotation.
Based on coding guidelines:
def preprocess_dicom_series(
dicom_path: Union[str, bytes],
modality: str,
-):
+) -> "MetaTensor":
"""
Preprocess a DICOM series based on modality.
Args:
dicom_path: Path to DICOM file or directory.
- modality: CT, MR, or MRI.
+ modality: Imaging modality. Supported values: "CT", "MR", "MRI" (case-insensitive).
Returns:
- Preprocessed image.
+ MetaTensor: Preprocessed image with intensity values normalized based on modality.
+
+ Raises:
+ TypeError: If modality is not a string.
+ ValueError: If modality is not one of the supported values.
"""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def preprocess_dicom_series( | |
| dicom_path: Union[str, bytes], | |
| modality: str, | |
| ): | |
| """ | |
| Preprocess a DICOM series based on modality. | |
| Args: | |
| dicom_path: Path to DICOM file or directory. | |
| modality: CT, MR, or MRI. | |
| Returns: | |
| Preprocessed image. | |
| """ | |
| if not isinstance(modality, str): | |
| raise TypeError("modality must be a string") | |
| 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("Unsupported modality") | |
| return transform(dicom_path) | |
| def preprocess_dicom_series( | |
| dicom_path: Union[str, bytes], | |
| modality: str, | |
| ) -> "MetaTensor": | |
| """ | |
| Preprocess a DICOM series based on modality. | |
| Args: | |
| dicom_path: Path to DICOM file or directory. | |
| modality: Imaging modality. Supported values: "CT", "MR", "MRI" (case-insensitive). | |
| Returns: | |
| MetaTensor: Preprocessed image with intensity values normalized based on modality. | |
| Raises: | |
| TypeError: If modality is not a string. | |
| ValueError: If modality is not one of the supported values. | |
| """ | |
| if not isinstance(modality, str): | |
| raise TypeError("modality must be a string") | |
| 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("Unsupported modality") | |
| return transform(dicom_path) |
🧰 Tools
🪛 Ruff (0.14.8)
59-59: Avoid specifying long messages outside the exception class
(TRY003)
68-68: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In monai/transforms/clinical_preprocessing.py around lines 44 to 70, the
function preprocess_dicom_series is missing a return type annotation and the
docstring lacks a Raises section and detailed return description; update the
function signature to include the appropriate return type (e.g., -> ImageType or
the actual pipeline output type used across the codebase), and expand the
docstring to add a "Raises" section listing TypeError for non-string modality
and ValueError for unsupported modalities, plus a "Returns" section that
specifies the concrete return type and what the returned preprocessed image
represents (shape, dtype or object type) and any side effects.
| else: | ||
| raise ValueError("Unsupported modality") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Bare exception message lacks detail.
The error message doesn't specify which modalities are supported, making it less helpful for users.
else:
- raise ValueError("Unsupported modality")
+ raise ValueError(f"Unsupported modality: {modality}. Supported values are: CT, MR, MRI")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| else: | |
| raise ValueError("Unsupported modality") | |
| SUPPORTED_MODALITIES = "CT, MR, MRI" | |
| # ... elsewhere in code ... | |
| else: | |
| raise ValueError(f"Unsupported modality: {modality}. Supported values are: {SUPPORTED_MODALITIES}") |
🧰 Tools
🪛 Ruff (0.14.8)
68-68: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In monai/transforms/clinical_preprocessing.py around lines 67-68, the ValueError
raised when hitting the else branch uses a generic message; update it to include
the unsupported modality value and list the supported modalities (e.g.,
f"Unsupported modality: {modality}. Supported modalities: {supported_list}") so
callers see what was provided and what options are valid; ensure the variable
names used match the function scope and that the supported modalities are a
stable list or tuple defined nearby.
1 participant
