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
9 changes: 5 additions & 4 deletions monai/transforms/io/array.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,11 @@ def __init__(
the_reader = look_up_option(_r.lower(), SUPPORTED_READERS)
try:
self.register(the_reader(*args, **kwargs))
except OptionalImportError:
warnings.warn(
f"required package for reader {_r} is not installed, or the version doesn't match requirement."
)
except OptionalImportError as e:
raise OptionalImportError(
f"Required package for reader {_r} is not installed, or the version doesn't match requirement. "
f"Please install the required package to use {_r}."
) from e
except TypeError: # the reader doesn't have the corresponding args/kwargs
warnings.warn(f"{_r} is not supported with the given parameters {args} {kwargs}.")
self.register(the_reader())
Expand Down
10 changes: 8 additions & 2 deletions tests/data/test_init_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from monai.data import ITKReader, NibabelReader, NrrdReader, NumpyReader, PILReader, PydicomReader
from monai.transforms import LoadImage, LoadImaged
from monai.utils import OptionalImportError
from tests.test_utils import SkipIfNoModule


Expand All @@ -26,8 +27,13 @@ def test_load_image(self):
self.assertIsInstance(instance2, LoadImage)

for r in ["NibabelReader", "PILReader", "ITKReader", "NumpyReader", "NrrdReader", "PydicomReader", None]:
inst = LoadImaged("image", reader=r)
self.assertIsInstance(inst, LoadImaged)
try:
inst = LoadImaged("image", reader=r)
self.assertIsInstance(inst, LoadImaged)
except OptionalImportError:
# Reader's backend package is not installed — expected in
# minimal-dependency environments after the fix for #7437.
pass
Comment on lines +30 to +36
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid swallowing OptionalImportError without assertions.

This except ...: pass weakens the test and can mask regressions. Assert the expected branch instead of silently continuing.

Suggested test-hardening patch
         for r in ["NibabelReader", "PILReader", "ITKReader", "NumpyReader", "NrrdReader", "PydicomReader", None]:
-            try:
-                inst = LoadImaged("image", reader=r)
-                self.assertIsInstance(inst, LoadImaged)
-            except OptionalImportError:
-                # Reader's backend package is not installed — expected in
-                # minimal-dependency environments after the fix for `#7437`.
-                pass
+            with self.subTest(reader=r):
+                try:
+                    inst = LoadImaged("image", reader=r)
+                except OptionalImportError as err:
+                    self.assertIsNotNone(r)
+                    self.assertIn(str(r), str(err))
+                else:
+                    self.assertIsInstance(inst, LoadImaged)

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
Verify each finding against the current code and only fix it if needed.

In `@tests/data/test_init_reader.py` around lines 30 - 36, The test currently
swallows OptionalImportError in the LoadImaged instantiation which can mask
regressions; change the test to explicitly assert the expected behavior instead
of pass: either use self.assertRaises(OptionalImportError, LoadImaged, "image",
reader=r) to assert the error is raised in minimal-dependency environments, or
keep the try block and in the except OptionalImportError as e branch assert
something specific about e (e.g., error message contains the backend name) so
the failing branch is tested rather than silently ignored; reference the
LoadImaged constructor and OptionalImportError in your change.


@SkipIfNoModule("nibabel")
@SkipIfNoModule("cupy")
Expand Down
40 changes: 40 additions & 0 deletions tests/transforms/test_load_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,5 +498,45 @@ 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 instead of silently falling back to another reader."""
from monai.utils import OptionalImportError

with self.assertRaises(OptionalImportError):
LoadImage(reader="ITKReader")

def test_specified_reader_not_installed_raises_mocked(self):
"""Mock test to verify OptionalImportError is raised (not just warned) 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":
# Return a class that raises OptionalImportError on instantiation,
# simulating a reader whose backend package is not installed
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):
LoadImage(reader="MockMissingReader")


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