Skip to content
Open
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
19 changes: 16 additions & 3 deletions monai/data/dataset_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,20 @@ def calculate_statistics(self, foreground_threshold: int = 0):
label, *_ = convert_data_type(data=label, output_type=torch.Tensor)

image_foreground = image[torch.where(label > foreground_threshold)]
if image_foreground.numel() == 0:
continue

voxel_max.append(image_foreground.max().item())
voxel_min.append(image_foreground.min().item())
voxel_ct += len(image_foreground)
voxel_sum += image_foreground.sum()
voxel_square_sum += torch.square(image_foreground).sum()

if voxel_ct == 0:
raise ValueError(
f"no foreground voxels found in any sample with foreground_threshold={foreground_threshold}; "
"set foreground_threshold=-1 to compute statistics over whole images."
)
Comment on lines +171 to +175

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Complete Google-style docstrings for the changed definitions.

  • monai/data/dataset_summary.py#L171-L175: document Raises: and Returns: None for calculate_statistics.
  • monai/data/dataset_summary.py#L221-L224: document Raises: and Returns: None for calculate_percentiles.
  • tests/data/test_dataset_summary.py#L103-L103: document the mixed foreground/background test.
  • tests/data/test_dataset_summary.py#L124-L124: document the all-background error test.

As per path instructions, Python definitions must document variables, return values, and raised exceptions in Google-style docstrings.

📍 Affects 2 files
  • monai/data/dataset_summary.py#L171-L175 (this comment)
  • monai/data/dataset_summary.py#L221-L224
  • tests/data/test_dataset_summary.py#L103-L103
  • tests/data/test_dataset_summary.py#L124-L124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@monai/data/dataset_summary.py` around lines 171 - 175, Complete the
Google-style docstrings for calculate_statistics and calculate_percentiles in
monai/data/dataset_summary.py, documenting their Raises behavior and Returns as
None. Also add descriptive docstrings for the mixed foreground/background and
all-background error tests in tests/data/test_dataset_summary.py at lines 103
and 124, respectively.

Source: Path instructions

self.data_max, self.data_min = max(voxel_max), min(voxel_min)
self.data_mean = (voxel_sum / voxel_ct).item()
self.data_std = (torch.sqrt(voxel_square_sum / voxel_ct - self.data_mean**2)).item()
Expand Down Expand Up @@ -204,11 +211,17 @@ def calculate_percentiles(
label, *_ = convert_data_type(data=label, output_type=torch.Tensor)

intensities = image[torch.where(label > foreground_threshold)].tolist()
if sampling_flag:
intensities = intensities[::interval]
all_intensities.append(intensities)
if intensities:
if sampling_flag:
intensities = intensities[::interval]
all_intensities.append(intensities)

all_intensities = list(chain(*all_intensities))
if not all_intensities:
raise ValueError(
f"no foreground voxels found in any sample with foreground_threshold={foreground_threshold}; "
"set foreground_threshold=-1 to compute statistics over whole images."
)
self.data_min_percentile, self.data_max_percentile = np.percentile(
all_intensities, [min_percentile, max_percentile]
)
Expand Down
30 changes: 30 additions & 0 deletions tests/data/test_dataset_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import nibabel as nib
import numpy as np
import torch

from monai.data import Dataset, DatasetSummary, create_test_image_3d
from monai.transforms import LoadImaged
Expand Down Expand Up @@ -99,6 +100,35 @@ def test_anisotropic_spacing(self):
target_spacing = calculator.get_target_spacing(anisotropic_threshold=4.0, percentile=20.0)
np.testing.assert_allclose(target_spacing, (1.0, 1.0, 1.8))

def test_mixed_foreground_and_background(self):
data = [
{"image": torch.rand(1, 4, 4), "label": torch.ones(1, 4, 4)},
{"image": torch.rand(1, 4, 4), "label": torch.zeros(1, 4, 4)},
]
image = torch.cat([d["image"] for d in data])
label = torch.cat([d["label"] for d in data])
expected = image[torch.where(label > 0)]

calculator = DatasetSummary(data, num_workers=0)
calculator.calculate_statistics()
np.testing.assert_allclose(calculator.data_mean, expected.mean().item(), rtol=1e-5, atol=1e-5)
np.testing.assert_allclose(calculator.data_std, expected.std(correction=0).item(), rtol=1e-5, atol=1e-5)
np.testing.assert_allclose(calculator.data_max, expected.max().item(), rtol=1e-5, atol=1e-5)
np.testing.assert_allclose(calculator.data_min, expected.min().item(), rtol=1e-5, atol=1e-5)

calculator.calculate_percentiles(sampling_flag=False)
np.testing.assert_allclose(calculator.data_min_percentile, np.percentile(expected, 0.5), rtol=1e-5, atol=1e-5)
np.testing.assert_allclose(calculator.data_max_percentile, np.percentile(expected, 99.5), rtol=1e-5, atol=1e-5)
np.testing.assert_allclose(calculator.data_median, np.median(expected), rtol=1e-5, atol=1e-5)

def test_all_background(self):
data = [{"image": torch.rand(1, 4, 4), "label": torch.zeros(1, 4, 4)}]
calculator = DatasetSummary(data, num_workers=0)
with self.assertRaisesRegex(ValueError, "foreground_threshold"):
calculator.calculate_statistics()
with self.assertRaisesRegex(ValueError, "foreground_threshold"):
calculator.calculate_percentiles()
Comment on lines +127 to +130

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the full remediation text.

The test must verify the recommendation foreground_threshold=-1. The current regex checks only foreground_threshold, so it would pass if the recommendation were removed.

Proposed assertion
-        with self.assertRaisesRegex(ValueError, "foreground_threshold"):
+        with self.assertRaisesRegex(ValueError, r"set foreground_threshold=-1"):

Apply this to both assertions.

📝 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
with self.assertRaisesRegex(ValueError, "foreground_threshold"):
calculator.calculate_statistics()
with self.assertRaisesRegex(ValueError, "foreground_threshold"):
calculator.calculate_percentiles()
with self.assertRaisesRegex(ValueError, r"set foreground_threshold=-1"):
calculator.calculate_statistics()
with self.assertRaisesRegex(ValueError, r"set foreground_threshold=-1"):
calculator.calculate_percentiles()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/data/test_dataset_summary.py` around lines 127 - 130, Update both
assertRaisesRegex checks around calculator.calculate_statistics() and
calculator.calculate_percentiles() to match the complete remediation text,
including the exact recommendation foreground_threshold=-1, rather than only the
parameter name.



if __name__ == "__main__":
unittest.main()
Loading