From a741d0bca8d3c18f62dc6f81e78ad088bfe76fb0 Mon Sep 17 00:00:00 2001 From: Sarah Date: Tue, 29 Jul 2025 12:03:20 +0300 Subject: [PATCH 1/4] Fix LoadImage to raise OptionalImportError when specified reader is not available - 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 This addresses issue #7437 where LoadImage would silently fall back to another reader when the specified reader (e.g., ITKReader) was not installed. Now it properly raises an OptionalImportError to make it clear that the requested reader is not available. Fixes: #7437 --- monai/transforms/io/array.py | 14 ++++++++++---- tests/transforms/test_load_image.py | 7 ++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/monai/transforms/io/array.py b/monai/transforms/io/array.py index 49b0665a90..635ebaa541 100644 --- a/monai/transforms/io/array.py +++ b/monai/transforms/io/array.py @@ -206,13 +206,19 @@ def __init__( if not has_built_in: the_reader = locate(f"{_r}") # search dotted path if the_reader is None: - the_reader = look_up_option(_r.lower(), SUPPORTED_READERS) + try: + the_reader = look_up_option(_r.lower(), SUPPORTED_READERS) + except ValueError: + # If the reader name is not recognized at all, raise OptionalImportError + raise OptionalImportError( + f"Cannot find reader '{_r}'. It may not be installed or recognized." + ) try: self.register(the_reader(*args, **kwargs)) - except OptionalImportError: - warnings.warn( + except OptionalImportError as e: + raise OptionalImportError( f"required package for reader {_r} is not installed, or the version doesn't match requirement." - ) + ) 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()) diff --git a/tests/transforms/test_load_image.py b/tests/transforms/test_load_image.py index 930a18f2ee..91da1e621d 100644 --- a/tests/transforms/test_load_image.py +++ b/tests/transforms/test_load_image.py @@ -28,7 +28,7 @@ from monai.data.meta_obj import set_track_meta from monai.data.meta_tensor import MetaTensor from monai.transforms import LoadImage -from monai.utils import optional_import +from monai.utils import OptionalImportError, optional_import from tests.test_utils import SkipIfNoModule, assert_allclose, skip_if_downloading_fails, testing_data_config itk, has_itk = optional_import("itk", allow_namespace_pkg=True) @@ -436,6 +436,11 @@ def test_my_reader(self): self.assertEqual(out.meta["name"], "my test") out = LoadImage(image_only=True, reader=_MiniReader, is_compatible=False)("test") self.assertEqual(out.meta["name"], "my test") + + def test_reader_not_installed_exception(self): + """test if an exception is raised when a specified reader is not installed""" + with self.assertRaises(OptionalImportError): + LoadImage(image_only=True, reader="NonExistentReader")("test") for item in (_MiniReader, _MiniReader(is_compatible=False)): out = LoadImage(image_only=True, reader=item)("test") self.assertEqual(out.meta["name"], "my test") From 3db882b00fe9d10fd7ad3ac72a899615fd80ff6c Mon Sep 17 00:00:00 2001 From: Sarah Date: Fri, 8 Aug 2025 00:12:14 +0300 Subject: [PATCH 2/4] Add raise_on_missing_reader flag to LoadImage for better error handling - Add raise_on_missing_reader parameter (defaults to False for backward compatibility) - When True, raises OptionalImportError if specified reader is not available - When False (default), issues warning and uses fallback readers - Update tests to verify new behavior - Addresses reviewer feedback on PR #8522 --- monai/transforms/io/array.py | 16 +++++++++++++--- tests/transforms/test_load_image.py | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/monai/transforms/io/array.py b/monai/transforms/io/array.py index 635ebaa541..ab27c551e1 100644 --- a/monai/transforms/io/array.py +++ b/monai/transforms/io/array.py @@ -138,6 +138,7 @@ def __init__( prune_meta_pattern: str | None = None, prune_meta_sep: str = ".", expanduser: bool = True, + raise_on_missing_reader: bool = False, *args, **kwargs, ) -> None: @@ -161,6 +162,8 @@ def __init__( in the metadata (nested dictionary). default is ".", see also :py:class:`monai.transforms.DeleteItemsd`. e.g. ``prune_meta_pattern=".*_code$", prune_meta_sep=" "`` removes meta keys that ends with ``"_code"``. expanduser: if True cast filename to Path and call .expanduser on it, otherwise keep filename as is. + raise_on_missing_reader: if True, raise OptionalImportError when a specified reader is not available, + otherwise attempt to use fallback readers. Default is False to maintain backward compatibility. args: additional parameters for reader if providing a reader name. kwargs: additional parameters for reader if providing a reader name. @@ -183,6 +186,7 @@ def __init__( self.pattern = prune_meta_pattern self.sep = prune_meta_sep self.expanduser = expanduser + self.raise_on_missing_reader = raise_on_missing_reader self.readers: list[ImageReader] = [] for r in SUPPORTED_READERS: # set predefined readers as default @@ -216,9 +220,15 @@ def __init__( try: self.register(the_reader(*args, **kwargs)) except OptionalImportError as e: - raise OptionalImportError( - f"required package for reader {_r} is not installed, or the version doesn't match requirement." - ) from e + if self.raise_on_missing_reader: + raise OptionalImportError( + f"required package for reader {_r} is not installed, or the version doesn't match requirement." + ) from e + else: + warnings.warn( + f"required package for reader {_r} is not installed, or the version doesn't match requirement. " + f"Will use fallback readers if available." + ) 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()) diff --git a/tests/transforms/test_load_image.py b/tests/transforms/test_load_image.py index 91da1e621d..4e72cab68c 100644 --- a/tests/transforms/test_load_image.py +++ b/tests/transforms/test_load_image.py @@ -15,6 +15,7 @@ import shutil import tempfile import unittest +import warnings from pathlib import Path import nibabel as nib @@ -446,6 +447,26 @@ def test_reader_not_installed_exception(self): self.assertEqual(out.meta["name"], "my test") out = LoadImage(image_only=True)("test", reader=_MiniReader(is_compatible=False)) self.assertEqual(out.meta["name"], "my test") + + def test_raise_on_missing_reader_flag(self): + """test raise_on_missing_reader flag behavior""" + # Test with flag enabled - should raise exception for unknown reader name + with self.assertRaises(OptionalImportError): + LoadImage(image_only=True, reader="UnknownReaderName", raise_on_missing_reader=True) + + # Test with flag disabled (default) - should also raise exception for unknown reader name + # because this is caught before the new flag logic + with self.assertRaises(OptionalImportError): + LoadImage(image_only=True, reader="UnknownReaderName", raise_on_missing_reader=False) + + # The flag primarily helps when reader is recognized but dependencies are missing + # Since we're in an ITK test environment, let's just verify the flag exists and doesn't break normal behavior + loader_with_flag = LoadImage(image_only=True, reader="ITKReader", raise_on_missing_reader=False) + loader_without_flag = LoadImage(image_only=True, reader="ITKReader") + + # Both should work since ITK is available in this test environment + self.assertIsInstance(loader_with_flag, LoadImage) + self.assertIsInstance(loader_without_flag, LoadImage) def test_itk_meta(self): """test metadata from a directory""" From dae4264a1aa5617a219ee0bcbac554c0cf39facd Mon Sep 17 00:00:00 2001 From: Sarah Date: Fri, 8 Aug 2025 00:13:10 +0300 Subject: [PATCH 3/4] Add raise_on_missing_reader flag to LoadImaged dictionary wrapper - Pass through raise_on_missing_reader parameter to underlying LoadImage - Update docstring to document the new parameter - Ensure consistent behavior between array and dictionary versions --- monai/transforms/io/dictionary.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/monai/transforms/io/dictionary.py b/monai/transforms/io/dictionary.py index be1e78db8a..1273b0ce0d 100644 --- a/monai/transforms/io/dictionary.py +++ b/monai/transforms/io/dictionary.py @@ -87,6 +87,7 @@ def __init__( prune_meta_sep: str = ".", allow_missing_keys: bool = False, expanduser: bool = True, + raise_on_missing_reader: bool = False, *args, **kwargs, ) -> None: @@ -123,6 +124,8 @@ def __init__( e.g. ``prune_meta_pattern=".*_code$", prune_meta_sep=" "`` removes meta keys that ends with ``"_code"``. allow_missing_keys: don't raise exception if key is missing. expanduser: if True cast filename to Path and call .expanduser on it, otherwise keep filename as is. + raise_on_missing_reader: if True, raise OptionalImportError when a specified reader is not available, + otherwise attempt to use fallback readers. Default is False to maintain backward compatibility. args: additional parameters for reader if providing a reader name. kwargs: additional parameters for reader if providing a reader name. """ @@ -136,6 +139,7 @@ def __init__( prune_meta_pattern, prune_meta_sep, expanduser, + raise_on_missing_reader, *args, **kwargs, ) From 2af050147ad072b69c8deb7933326805cc65f97d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 7 Aug 2025 21:18:30 +0000 Subject: [PATCH 4/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/transforms/test_load_image.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/transforms/test_load_image.py b/tests/transforms/test_load_image.py index 4e72cab68c..30d34b37bc 100644 --- a/tests/transforms/test_load_image.py +++ b/tests/transforms/test_load_image.py @@ -15,7 +15,6 @@ import shutil import tempfile import unittest -import warnings from pathlib import Path import nibabel as nib @@ -447,23 +446,23 @@ def test_reader_not_installed_exception(self): self.assertEqual(out.meta["name"], "my test") out = LoadImage(image_only=True)("test", reader=_MiniReader(is_compatible=False)) self.assertEqual(out.meta["name"], "my test") - + def test_raise_on_missing_reader_flag(self): """test raise_on_missing_reader flag behavior""" # Test with flag enabled - should raise exception for unknown reader name with self.assertRaises(OptionalImportError): LoadImage(image_only=True, reader="UnknownReaderName", raise_on_missing_reader=True) - + # Test with flag disabled (default) - should also raise exception for unknown reader name # because this is caught before the new flag logic with self.assertRaises(OptionalImportError): LoadImage(image_only=True, reader="UnknownReaderName", raise_on_missing_reader=False) - + # The flag primarily helps when reader is recognized but dependencies are missing # Since we're in an ITK test environment, let's just verify the flag exists and doesn't break normal behavior loader_with_flag = LoadImage(image_only=True, reader="ITKReader", raise_on_missing_reader=False) loader_without_flag = LoadImage(image_only=True, reader="ITKReader") - + # Both should work since ITK is available in this test environment self.assertIsInstance(loader_with_flag, LoadImage) self.assertIsInstance(loader_without_flag, LoadImage)