Fix LoadImage to raise OptionalImportError when specified reader is not available

[its-serah]

Description

This PR fixes issue #7437 by making LoadImage raise an OptionalImportError when a specified reader is not installed, instead of silently falling back to another reader.

Changes

• Modified LoadImage.__init__ to catch ValueError from look_up_option when reader name is not recognized
• Raise OptionalImportError instead of just warning when specified reader is not installed
• Added test case to verify the new behavior

Why this is needed

Previously, when a user specified LoadImage(reader='ITKReader') without ITK installed, it would just warn and use PILReader instead. This could lead to confusion and unexpected behavior. Now it properly raises an OptionalImportError to make it clear that the requested reader is not available.

Fixes #7437

[coderabbitai[bot]]

Walkthrough

Added a new constructor parameter raise_on_missing_reader: bool = False to LoadImage in monai/transforms/io/array.py and stored it on the instance. Reader resolution now wraps lookup and instantiation in try/except: when a user-specified reader name cannot be resolved or imported, an OptionalImportError is raised if raise_on_missing_reader is True; otherwise the code warns and continues attempting fallback readers. Exception handling was added/adjusted for string-based readers, reader classes, and reader instances (including TypeError where applicable). Minor import reordering was applied. Two tests were added in tests/transforms/test_load_image.py to validate raise-on-missing-reader behavior and failure cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify consistent propagation and use of self.raise_on_missing_reader in all reader-resolution branches (string, class, instance).
• Confirm correct exception types and messaging when re-raising OptionalImportError vs. issuing warnings.
• Review the new tests test_reader_not_installed_exception and test_raise_on_missing_reader_flag for correctness and coverage.
• Check that default behavior (raise_on_missing_reader=False) preserves existing fallback logic and warning semantics.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately summarizes the main change: raising OptionalImportError when a specified reader is unavailable.
Description check	✅ Passed	Description covers the fix, changes made, and rationale. Follows template with clear issue reference and change summary.
Linked Issues check	✅ Passed	PR implements the core requirement from #7437: raises OptionalImportError when specified reader is unavailable, while preserving backward-compatible fallback behavior via raise_on_missing_reader flag.
Out of Scope Changes check	✅ Passed	All changes align with #7437: LoadImage modifications, error handling, fallback logic, and comprehensive tests for both exception and fallback behaviors are directly scoped to the issue.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• [ ] 📝 Generate docstrings

🧪 Generate unit tests (beta)

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 16b0ba6f03b2dbf535278549f6012070458cc6a0 and ae97ff7f80968a3b3a931ffe0b112bea05ba46c7.

📒 Files selected for processing (1)

• tests/transforms/test_load_image.py (3 hunks)

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

• tests/transforms/test_load_image.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)

• GitHub Check: min-dep-py3 (3.12)
• GitHub Check: min-dep-pytorch (2.8.0)
• GitHub Check: min-dep-py3 (3.9)
• GitHub Check: min-dep-os (macOS-latest)
• GitHub Check: min-dep-pytorch (2.7.1)
• GitHub Check: min-dep-py3 (3.11)
• GitHub Check: min-dep-py3 (3.10)
• GitHub Check: min-dep-pytorch (2.5.1)
• GitHub Check: min-dep-pytorch (2.6.0)
• GitHub Check: min-dep-os (ubuntu-latest)
• GitHub Check: min-dep-os (windows-latest)
• GitHub Check: quick-py3 (macOS-latest)
• GitHub Check: flake8-py3 (pytype)
• GitHub Check: build-docs
• GitHub Check: flake8-py3 (codeformat)
• GitHub Check: quick-py3 (ubuntu-latest)
• GitHub Check: flake8-py3 (mypy)
• GitHub Check: quick-py3 (windows-latest)
• GitHub Check: packaging

---

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 and usage tips.}

[ericspod]

Hi @its-serah thanks for the contribution. As you see from the failed tests the expectation of the reader is to not raise an exception if an optional package isn't found, and this is correct in the cases when a fallback reader does exist for some formats. If we raise an exception whenever a reader can't be loaded then this fallback behaviour can't happen. I would suggest that we add a flag as a member to the class to enable the exception behaviour, but whose default state retains the existing behaviour. We would also need tests to check that turning this on correctly raises exceptions. What do you think? There's interest in the associated issue so I'm keen to find a solution that works for everyone. Thanks!

[its-serah]

Hi @ericspod thanks for the excellent feedback! I've implemented exactly what you suggested. I added a raise_on_missing_reader flag to both LoadImage and LoadImaged classes that defaults to False to maintain existing behavior and backward compatibility. When set to True, it raises OptionalImportError for missing readers. When False (default), it preserves the current fallback behavior with warnings. I also added comprehensive tests to verify the flag correctly raises exceptions when enabled. This gives users control while maintaining the important fallback functionality for existing codebases. The failed tests should now pass since the default behavior is unchanged. What do you think of this approach?

[ericspod]

Hi @its-serah it looks better now, but I think the two raises need to be guarded by the same mechanism. Other than that it looks good though you'll have to fix your DCO issue and the formatting issue (./runtests.sh --autofix will do it). Thanks!

[its-serah]

Hi @ericspod thanks for the feedback! I've addressed both issues you mentioned:

Fixed the guarding mechanism: Both raise OptionalImportError statements now use the same raise_on_missing_reader flag mechanism for consistent behavior.

Fixed DCO and formatting: Added proper DCO sign-off and applied code formatting fixes using isort, black, and ruff.

The changes ensure that both exception cases (unrecognized reader name and missing dependencies) are handled consistently through the same flag. When raise_on_missing_reader=False, both cases will show warnings and allow fallback behavior. When True, both will raise OptionalImportError.

[ericspod]

Hi @its-serah thanks for continuing to work on this, I think it's much better with some minor suggestions. The tests are failing however so please look again and what your tests are testing to see what the issue is. The DCO still doesn't seem happy but we can sort that last and just force it to pass if needed.

[its-serah]

Hey @ericspod

Thank you for the code review feedback! I've addressed both of your suggestions in the latest commits:

Refactored duplicate error messaging: • Lines 217-225: Extracted the common message f"Cannot find reader '{_r}'. It may not be installed or recognized." into a variable and reused it for both the exception and warning paths • Lines 230-238: Similarly extracted f"Required package for reader {_r} is not installed, or the version doesn't match requirement." to eliminate duplication

Fixed the failing test: • Updated test_raise_on_missing_reader_flag to correctly verify the intended behavior • The test now properly expects warnings (not exceptions) when raise_on_missing_reader=False • Added proper warning verification using warnings.catch_warnings()

The changes maintain backward compatibility while implementing the cleaner code structure you suggested. I've also verified that the functionality works as expected with custom tests.

The CI failures should now be resolved since the test logic correctly matches the implementation behavior. Please let me know if you need any clarifications or have additional feedback!

[ericspod]

Hi @its-serah thanks for the update, I think this addresses the suggested code I had. The tests are failing however since your tests are attempting to load a non-existent file. There's also a few things from Coderabbit but we're just about there.

[dzenanz]

@its-serah are you fine with me finishing this PR?

[its-serah]

@dzenanz No that's fine I'll handle it, thanks for the reminder!

[its-serah]

pre-commit.ci run

[its-serah]

PR Updated - Ready for Review! Added raise_on_missing_reader flag to LoadImage/LoadImaged. All CodeRabbit feedback addressed, tests passing, backward compatible. Defaults to False for graceful fallback behavior.