fix(data): avoid divide-by-zero in pydicom affine for single-slice volumes

[SID-6921]

Summary:

• fix PydicomReader._get_affine to skip lastImagePositionPatient z-step derivation when there is only one slice
• add regression tests for single-slice and multi-slice metadata paths

Why:
For single-slice 3D DICOM segmentation metadata, n == 1 caused division by zero in affine z-step computation.

Validation:

• python -m pytest tests/data/test_init_reader.py -k pydicom_reader_get_affine -q
• python -m pre_commit run --files monai/data/image_reader.py tests/data/test_init_reader.py

Closes #8925

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

tests/data/test_init_reader.py adds MetaKeys import, changes the Pydicom skip decorator to use pydicom, and adds two PydicomReader._get_affine tests for single-slice and multi-slice metadata. Both tests build lastImagePositionPatient and MetaKeys.SPATIAL_SHAPE, call _get_affine(..., lps_to_ras=False), and assert the Z-axis affine entries.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The linked bug fix in monai/data/image_reader.py is missing; only regression tests were added.	Add the n > 1 guard to DICOMReader._get_affine in monai/data/image_reader.py and keep the single- and multi-slice regression tests.
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the intended fix for single-slice pydicom affine handling.
Description check	✅ Passed	It covers the summary, rationale, validation, and issue reference, though it doesn't use the repository's exact template.
Out of Scope Changes check	✅ Passed	The only changes are regression tests and a module-name skip tweak, both relevant to the pydicom fix.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

monai/data/image_reader.py (1)

729-771: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document the new _get_affine fallback.

This definition changed, but its docstring still omits a Returns section and does not say that lastImagePositionPatient is ignored unless MetaKeys.SPATIAL_SHAPE[-1] > 1. Please document that single-slice inputs keep the default third column. As per path instructions, "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."

🤖 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/image_reader.py` around lines 729 - 771, The _get_affine docstring
is missing the new fallback behavior and return documentation. Update the
Google-style docstring for _get_affine to include a Returns section for the
affine matrix, and note that lastImagePositionPatient is only used when
MetaKeys.SPATIAL_SHAPE[-1] > 1; for single-slice inputs the default third column
remains unchanged.

Source: Path instructions

🤖 Prompt for all review comments with 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.

Inline comments:
In `@tests/data/test_init_reader.py`:
- Around line 104-135: Add Google-style docstrings to the two new test methods
in PydicomReader’s test class:
test_pydicom_reader_get_affine_single_slice_with_last_position and
test_pydicom_reader_get_affine_multi_slice_uses_last_position. Briefly describe
the purpose of each test, the key metadata variables they set up, and the
expected affine assertions in the Args/Returns/Raises sections as appropriate,
matching the project’s docstring requirements for test definitions.
- Around line 120-134: The PydicomReader._get_affine multi-slice test is too
weak because the current lastImagePositionPatient and MetaKeys.SPATIAL_SHAPE
values produce the same affine result as the default branch, so it does not
prove the special path executed. Update
test_pydicom_reader_get_affine_multi_slice_uses_last_position to use a
lastImagePositionPatient value and spatial shape that yield a non-default k3
term, then assert that affine[2, 2] (or the full third column) reflects the
branch-specific result so the test fails if _get_affine skips the
lastImagePositionPatient logic.

---

Outside diff comments:
In `@monai/data/image_reader.py`:
- Around line 729-771: The _get_affine docstring is missing the new fallback
behavior and return documentation. Update the Google-style docstring for
_get_affine to include a Returns section for the affine matrix, and note that
lastImagePositionPatient is only used when MetaKeys.SPATIAL_SHAPE[-1] > 1; for
single-slice inputs the default third column remains unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2ddf1ea8-6e46-466f-b496-8d91deca2125

📥 Commits

Reviewing files that changed from the base of the PR and between 4ba89bd and 4a51549.

📒 Files selected for processing (2)

• monai/data/image_reader.py
• tests/data/test_init_reader.py

[SID-6921]

Thanks for the heads-up. I synced with current dev and removed the duplicate change in monai/data/image_reader.py from this PR. I kept the regression tests in ests/data/test_init_reader.py (single-slice and multi-slice affine metadata coverage), which still add value. Local validation passes for the updated PR scope.

[SID-6921]

Update done: I rebased the PR branch onto current dev and force-pushed it so the duplicate image_reader.py fix is no longer part of this PR. The PR now keeps only the added regression coverage in ests/data/test_init_reader.py (single-slice and multi-slice affine metadata cases, guarded by optional Pydicom availability).

[ericspod]

Thanks @SID-6921 I had minor comments about the module name and the Coderabbit comment is worth looking at.

[SID-6921]

Thanks for the review comments. I applied the module-name fix and force-pushed: all SkipIfNoModule uses for pydicom in this test file are now lowercase (pydicom), including the two new affine regression tests. Local pytest and pre-commit passed after this change.

[Copilot]

Pull request overview

This PR fixes a ZeroDivisionError in PydicomReader._get_affine when handling single-slice (n=1) 3D DICOM metadata that includes lastImagePositionPatient, and adds regression tests to cover the single-slice and multi-slice affine paths.

Changes:

• Guard z-step (slice direction) affine derivation from lastImagePositionPatient to avoid dividing by (n - 1) when n == 1.
• Add regression tests for _get_affine with lastImagePositionPatient for both single-slice and multi-slice shapes.
• Standardize SkipIfNoModule usage for pydicom by using the correct import name ("pydicom").

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

File	Description
monai/data/image_reader.py	Prevents divide-by-zero in _get_affine when computing slice step from lastImagePositionPatient for single-slice volumes.
tests/data/test_init_reader.py	Adds regression tests for _get_affine single-slice and multi-slice paths; updates pydicom module skip guard.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.