Address code review feedback: refactor error messaging and fix test

- Refactor duplicate error messaging for missing reader handling as suggested by @ericspod
- Extract common error message strings to avoid duplication
- Fix test to properly verify raise_on_missing_reader=False behavior
- Update test to expect warnings instead of exceptions when flag is disabled
- Maintain backward compatibility with proper error handling

Author: its-serah

monai/transforms/io/array.py

@@ -214,28 +214,20 @@ def __init__(
                         the_reader = look_up_option(_r.lower(), SUPPORTED_READERS)
                     except ValueError:
                         # If the reader name is not recognized at all, raise OptionalImportError
+                        msg = f"Cannot find reader '{_r}'. It may not be installed or recognized."
                         if self.raise_on_missing_reader:
-                            raise OptionalImportError(
-                                f"Cannot find reader '{_r}'. It may not be installed or recognized."
-                            )
+                            raise OptionalImportError(msg)
                         else:
-                            warnings.warn(
-                                f"Cannot find reader '{_r}'. It may not be installed or recognized. "
-                                f"Will use fallback readers if available."
-                            )
+                            warnings.warn(f"{msg} Will use fallback readers if available.")
                             continue
                 try:
                     self.register(the_reader(*args, **kwargs))
                 except OptionalImportError as e:
+                    msg = f"Required package for reader {_r} is not installed, or the version doesn't match requirement."
                     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
+                        raise OptionalImportError(msg) 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."
-                        )
+                        warnings.warn(f"{msg} 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())

test_my_changes.py

@@ -0,0 +1,55 @@
+#!/usr/bin/env python3
+
+"""Test to verify the changes to LoadImage raise_on_missing_reader flag work correctly."""
+
+import warnings
+from monai.transforms import LoadImage
+from monai.utils import OptionalImportError
+
+def test_raise_on_missing_reader():
+    """Test the raise_on_missing_reader flag behavior."""
+    print("Testing LoadImage raise_on_missing_reader flag...")
+    
+    # Test 1: Unknown reader with flag enabled - should raise OptionalImportError
+    print("Test 1: Unknown reader with raise_on_missing_reader=True")
+    try:
+        LoadImage(reader="UnknownReader", raise_on_missing_reader=True)
+        print("FAIL: Expected OptionalImportError but didn't get one")
+        return False
+    except OptionalImportError as e:
+        print(f"PASS: Got expected OptionalImportError: {e}")
+    
+    # Test 2: Unknown reader with flag disabled - should warn but not raise
+    print("\nTest 2: Unknown reader with raise_on_missing_reader=False")  
+    try:
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+            loader = LoadImage(reader="UnknownReader", raise_on_missing_reader=False)
+            if w and "UnknownReader" in str(w[0].message):
+                print(f"PASS: Got expected warning: {w[0].message}")
+            else:
+                print("WARNING: Warning message may have been different than expected")
+        print("PASS: LoadImage instance created successfully with fallback behavior")
+    except OptionalImportError as e:
+        print(f"FAIL: Unexpected OptionalImportError with flag disabled: {e}")
+        return False
+    except Exception as e:
+        print(f"FAIL: Unexpected error: {e}")
+        return False
+    
+    # Test 3: Valid reader - should work regardless of flag
+    print("\nTest 3: Valid reader (PILReader) with both flag settings")
+    try:
+        loader1 = LoadImage(reader="PILReader", raise_on_missing_reader=True)
+        loader2 = LoadImage(reader="PILReader", raise_on_missing_reader=False)
+        print("PASS: Both loaders created successfully with valid reader")
+    except Exception as e:
+        print(f"FAIL: Unexpected error with valid reader: {e}")
+        return False
+    
+    print("\nAll tests passed!")
+    return True
+
+if __name__ == "__main__":
+    success = test_raise_on_missing_reader()
+    exit(0 if success else 1)

tests/transforms/test_load_image.py

@@ -454,13 +454,16 @@ def test_raise_on_missing_reader_flag(self):
         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)
+        # Test with flag disabled - should warn but not raise exception for unknown reader name
+        # This should succeed and create the loader with fallback behavior
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+            loader_with_fallback = LoadImage(image_only=True, reader="UnknownReaderName", raise_on_missing_reader=False)
+            self.assertIsInstance(loader_with_fallback, LoadImage)
+            # Should have produced a warning about the unknown reader
+            self.assertTrue(any("Cannot find reader 'UnknownReaderName'" in str(warning.message) for warning in w))
 
-        # 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
+        # The flag should work properly with valid readers too
         loader_with_flag = LoadImage(image_only=True, reader="ITKReader", raise_on_missing_reader=False)
         loader_without_flag = LoadImage(image_only=True, reader="ITKReader")