-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Raise exception when user-specified reader package is not installed #8771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -498,5 +498,47 @@ def test_correct(self, input_param, expected_shape, track_meta): | |
| self.assertFalse(hasattr(r, "affine")) | ||
|
|
||
|
|
||
| class TestLoadImageReaderNotInstalled(unittest.TestCase): | ||
| """Test that specifying a reader whose required package is not installed raises an error. | ||
|
|
||
| Addresses https://github.com/Project-MONAI/MONAI/issues/7437 | ||
| """ | ||
|
|
||
| @unittest.skipIf(has_itk, "test requires itk to NOT be installed") | ||
| def test_specified_reader_not_installed_raises(self): | ||
| """When a user explicitly specifies a reader that is not installed, LoadImage should raise | ||
| an OptionalImportError with an install hint instead of silently falling back.""" | ||
| from monai.utils import OptionalImportError | ||
|
|
||
| with self.assertRaises(OptionalImportError) as ctx: | ||
| LoadImage(reader="ITKReader") | ||
| self.assertIn("pip install itk", str(ctx.exception)) | ||
|
|
||
| def test_specified_reader_not_installed_raises_mocked(self): | ||
| """Mock test to verify OptionalImportError is raised with original message preserved | ||
| when a user-specified reader's required package is not installed.""" | ||
| from unittest.mock import patch | ||
|
|
||
| from monai.utils import OptionalImportError | ||
|
|
||
| _original = __import__("monai.transforms.io.array", fromlist=["optional_import"]).optional_import | ||
|
|
||
| def _mock_optional_import(module, name="", *args, **kwargs): | ||
| if name == "MockMissingReader": | ||
|
|
||
| class _Unavailable: | ||
| def __init__(self, *a, **kw): | ||
| raise OptionalImportError("mock package is not installed") | ||
|
|
||
| return _Unavailable, True | ||
| return _original(module, *args, name=name, **kwargs) | ||
|
|
||
| with patch("monai.transforms.io.array.optional_import", side_effect=_mock_optional_import): | ||
| with self.assertRaises(OptionalImportError) as ctx: | ||
| LoadImage(reader="MockMissingReader") | ||
| # Original error message should be preserved in the re-raised exception | ||
| self.assertIn("mock package is not installed", str(ctx.exception)) | ||
|
|
||
|
Comment on lines
+501
to
+541
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Docstring guideline is only partially satisfied for newly added definitions. New definitions in this block are missing full Google-style docstrings (notably nested As per coding guidelines, "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 |
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid swallowing
OptionalImportErrorwithout assertions.This
except ...: passweakens the test and can mask regressions. Assert the expected branch instead of silently continuing.Suggested test-hardening patch
As per coding guidelines, "Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Ensure new or modified definitions will be covered by existing or new unit tests."
🤖 Prompt for AI Agents