Raise OptionalImportError when user-specified reader is not installed

[vandhanaanatarajan-oss]

(fixes #7437)

Fixes

Description

A few sentences describing the changes proposed in this pull request.

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Breaking change (fix or new feature that would cause existing functionality to change).
• New tests added to cover the changes.
• Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
• Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
• In-line docstrings updated.
• Documentation updated, tested make html command in the docs/ folder.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c7e9de29-34f8-4052-b280-80afe0251445

📥 Commits

Reviewing files that changed from the base of the PR and between aedd0bb and f3bd473.

📒 Files selected for processing (2)

• monai/transforms/io/array.py
• tests/transforms/test_load_image.py

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/transforms/test_load_image.py
• monai/transforms/io/array.py

---

📝 Walkthrough

Walkthrough

LoadImage now raises OptionalImportError when an explicitly selected reader lacks its dependency. The original exception remains the cause. A test verifies this behavior with ITKReader. Dependency-based skip decorators were removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description includes the template but retains placeholder text and incorrectly leaves the added-tests and docstring items unchecked.	Replace placeholder text with a specific change summary and mark the added tests and updated inline docstrings accurately.
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.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 clearly describes the main behavior change for user-specified readers that are not installed.
Linked Issues check	✅ Passed	The implementation raises OptionalImportError for an unavailable explicitly selected reader and adds a test for issue #7437.
Out of Scope Changes check	✅ Passed	The source and test changes are directly related to the linked issue and the stated LoadImage behavior change.

✨ 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]

🧹 Nitpick comments (2)

monai/transforms/io/array.py (1)

212-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the new constructor exception.

LoadImage.__init__ now raises OptionalImportError when an explicitly selected reader lacks its dependency. Add a Google-style Raises section to the constructor docstring.

Suggested documentation update

         Args:
             kwargs: additional parameters for reader if providing a reader name.
 
+        Raises:
+            OptionalImportError: If an explicitly selected reader dependency is unavailable.
+
         Note:

As per path instructions, Python definitions must use Google-style docstrings that document raised exceptions.

🤖 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/transforms/io/array.py` around lines 212 - 215, Update the
LoadImage.__init__ docstring to include a Google-style Raises section
documenting OptionalImportError when an explicitly selected reader’s required
dependency is unavailable or incompatible; leave the constructor behavior
unchanged.

Source: Path instructions

tests/transforms/test_load_image.py (1)

225-231: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert the preserved exception cause.

Line [230] checks only the exception type. The test would still pass if Line [215] stopped chaining the original error. Store the mocked exception, capture assertRaises, and assert context.exception.__cause__ is original_error.

Suggested test update

     def test_reader_not_installed_raises(self):
+        """Verify that an unavailable explicit reader raises OptionalImportError."""
         from unittest.mock import patch
         from monai.utils import OptionalImportError
 
-        with patch("monai.data.ITKReader", side_effect=OptionalImportError("itk not installed")):
-            with self.assertRaises(OptionalImportError):
+        original_error = OptionalImportError("itk not installed")
+        with patch("monai.data.ITKReader", side_effect=original_error):
+            with self.assertRaises(OptionalImportError) as context:
                 LoadImage(reader="ITKReader")
+        self.assertIs(context.exception.__cause__, original_error)

🤖 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/transforms/test_load_image.py` around lines 225 - 231, Update
test_reader_not_installed_raises to store the OptionalImportError instance
passed as the ITKReader patch side effect, capture the assertRaises context, and
assert that context.exception.__cause__ is the same original error. Keep the
existing reader setup and exception-type assertion intact.

🤖 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.

Nitpick comments:
In `@monai/transforms/io/array.py`:
- Around line 212-215: Update the LoadImage.__init__ docstring to include a
Google-style Raises section documenting OptionalImportError when an explicitly
selected reader’s required dependency is unavailable or incompatible; leave the
constructor behavior unchanged.

In `@tests/transforms/test_load_image.py`:
- Around line 225-231: Update test_reader_not_installed_raises to store the
OptionalImportError instance passed as the ITKReader patch side effect, capture
the assertRaises context, and assert that context.exception.__cause__ is the
same original error. Keep the existing reader setup and exception-type assertion
intact.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c2b1874d-064e-4f50-8a7e-c5e6c8552e2e

📥 Commits

Reviewing files that changed from the base of the PR and between 8690ae7 and aedd0bb.

📒 Files selected for processing (2)

• monai/transforms/io/array.py
• tests/transforms/test_load_image.py