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
8 changes: 8 additions & 0 deletions monai/data/image_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,14 @@ def _get_affine(self, metadata: dict, lps_to_ras: bool = True):
"""
affine: np.ndarray = np.eye(4)
if not ("00200037" in metadata and "00200032" in metadata):
warnings.warn(
"PydicomReader: ImageOrientationPatient (0020,0037) and/or "
"ImagePositionPatient (0020,0032) tags are missing, so the affine "
"matrix cannot be derived and defaults to the identity. The image "
"orientation and spacing may be incorrect (e.g. for multi-frame "
"Enhanced DICOM); consider using ITKReader for such files.",
stacklevel=2,
)
return affine
# "00200037" is the tag of `ImageOrientationPatient`
rx, ry, rz, cx, cy, cz = metadata["00200037"]["Value"]
Expand Down
44 changes: 44 additions & 0 deletions tests/data/test_pydicom_reader.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Copyright (c) MONAI Consortium
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
# http://www.apache.org/licenses/LICENSE-2.0
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

import unittest

import numpy as np

from monai.data import PydicomReader
from tests.test_utils import SkipIfNoModule


@SkipIfNoModule("pydicom")
class TestPydicomReaderAffine(unittest.TestCase):
def test_missing_orientation_tags_warns_and_returns_identity(self):
# Without ImageOrientationPatient (0020,0037) and ImagePositionPatient
# (0020,0032) the affine cannot be derived. The reader falls back to the
# identity matrix; regression test for #8468 ensures this is no longer
# silent so users know orientation/spacing may be wrong.
reader = PydicomReader()
with self.assertWarns(UserWarning):
affine = reader._get_affine({})
np.testing.assert_array_equal(affine, np.eye(4))
Comment on lines +24 to +32

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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add docstring per coding guidelines.

Google-style docstring should describe the test purpose, including what is being verified and expected behavior.

📝 Proposed docstring
 def test_missing_orientation_tags_warns_and_returns_identity(self):
+    """Verify UserWarning is emitted when both orientation and position tags are absent.
+    
+    The affine should default to identity matrix when metadata is empty.
+    """
     # Without ImageOrientationPatient (0020,0037) and ImagePositionPatient
🤖 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_pydicom_reader.py` around lines 24 - 32, The test method
test_missing_orientation_tags_warns_and_returns_identity is missing a
Google-style docstring. Add a docstring to this method that describes its
purpose, explains what behavior is being tested (that missing orientation tags
trigger a UserWarning and return an identity matrix affine), and clarifies the
regression test context for issue `#8468`. The docstring should be placed
immediately after the method definition line and before any code logic.

Source: Coding guidelines


def test_partial_orientation_tags_warns(self):
# Only one of the two required tags present is still insufficient.
reader = PydicomReader()
metadata = {"00200037": {"Value": [1, 0, 0, 0, 1, 0]}} # orientation only
with self.assertWarns(UserWarning):
affine = reader._get_affine(metadata)
np.testing.assert_array_equal(affine, np.eye(4))
Comment on lines +34 to +40

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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add docstring per coding guidelines.

Google-style docstring should describe the test purpose, including what is being verified and expected behavior.

📝 Proposed docstring
 def test_partial_orientation_tags_warns(self):
+    """Verify UserWarning is emitted when only orientation tag is present.
+    
+    The affine should default to identity when position tag (0020,0032) is missing.
+    """
     # Only one of the two required tags present is still insufficient.
📝 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 test_partial_orientation_tags_warns(self):
# Only one of the two required tags present is still insufficient.
reader = PydicomReader()
metadata = {"00200037": {"Value": [1, 0, 0, 0, 1, 0]}} # orientation only
with self.assertWarns(UserWarning):
affine = reader._get_affine(metadata)
np.testing.assert_array_equal(affine, np.eye(4))
def test_partial_orientation_tags_warns(self):
"""Verify UserWarning is emitted when only orientation tag is present.
The affine should default to identity when position tag (0020,0032) is missing.
"""
# Only one of the two required tags present is still insufficient.
reader = PydicomReader()
metadata = {"00200037": {"Value": [1, 0, 0, 0, 1, 0]}} # orientation only
with self.assertWarns(UserWarning):
affine = reader._get_affine(metadata)
np.testing.assert_array_equal(affine, np.eye(4))
🤖 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_pydicom_reader.py` around lines 34 - 40, The
test_partial_orientation_tags_warns method is missing a Google-style docstring.
Add a docstring immediately after the method definition that describes the test
purpose, explains what is being verified (that having only one of the two
required orientation tags results in a UserWarning), and documents the expected
behavior (that the affine matrix returned is the identity matrix). Follow
Google-style docstring conventions with a summary line and additional details as
needed.

Source: Coding guidelines



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