Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added docs/clinical_dicom_workflow.pdf
Binary file not shown.
Binary file added monai/docs/clinical_dicom_workflow.pdf
Binary file not shown.
45 changes: 45 additions & 0 deletions monai/tests/test_clinical_preprocessing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import numpy as np

from monai.transforms import ScaleIntensityRange, NormalizeIntensity
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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

Comment on lines +6 to +28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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)
Comment on lines +30 to +45
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Test doesn't validate the pipeline function.

Same issue as above—this validates NormalizeIntensity but not get_mri_preprocessing_pipeline() or preprocess_dicom_series().

70 changes: 70 additions & 0 deletions monai/transforms/clinical_preprocessing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
from typing import Union

from monai.transforms import (
Compose,
LoadImage,
EnsureChannelFirst,
ScaleIntensityRange,
NormalizeIntensity,
)


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,
),
]
)
Comment on lines +12 to +28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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),
]
)
Comment on lines +31 to +41
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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")
Comment on lines +67 to +68
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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.


return transform(dicom_path)
Comment on lines +44 to +70
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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.

Loading