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
20 changes: 18 additions & 2 deletions monai/data/image_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,16 +107,32 @@ def resolve_writer(ext_name, error_if_not_found=True) -> Sequence:
fmt = fmt[1:]
avail_writers = []
default_writers = SUPPORTED_WRITERS.get(EXT_WILDCARD, ())
import re
errors = []
for _writer in look_up_option(fmt, SUPPORTED_WRITERS, default=default_writers):
try:
_writer() # this triggers `monai.utils.module.require_pkg` to check the system availability
avail_writers.append(_writer)
except OptionalImportError:
except OptionalImportError as e:
errors.append(str(e))
continue
except Exception: # other writer init errors indicating it exists
avail_writers.append(_writer)
if not avail_writers and error_if_not_found:
raise OptionalImportError(f"No ImageWriter backend found for {fmt}.")
required_pkgs = []
for err in errors:
match = re.search(r"required package `([^`]+)`", err)
if match:
pkg = match.group(1)
pkg = "pillow" if pkg == "PIL" else pkg
pkg = "pynrrd" if pkg == "nrrd" else pkg
if pkg not in required_pkgs:
required_pkgs.append(pkg)

msg = f"No ImageWriter backend found for {fmt}."
if required_pkgs:
msg += f" Please install: {' or '.join(required_pkgs)}."
raise OptionalImportError(msg)
writer_tuple = ensure_tuple(avail_writers)
SUPPORTED_WRITERS[fmt] = writer_tuple
return writer_tuple
Expand Down
13 changes: 9 additions & 4 deletions monai/transforms/io/array.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,18 @@ def __init__(
the_reader = look_up_option(_r.lower(), SUPPORTED_READERS)
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}.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the file and show the relevant section around line 217
ls -la monai/transforms/io/array.py
python3 - <<'PY'
import itertools, pathlib
p = pathlib.Path("monai/transforms/io/array.py")
lines = p.read_text().splitlines()
start, end = 200, 240
for i in range(start-1, min(end, len(lines))):
    print(f"{i+1:4d}: {lines[i]}")
PY

# Find all warnings.warn occurrences in this file
rg -n "warnings\.warn\s*\(" monai/transforms/io/array.py

Repository: Project-MONAI/MONAI

Length of output: 2624


🏁 Script executed:

#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib
p = pathlib.Path("monai/transforms/io/array.py")
lines = p.read_text().splitlines()
# show more context to identify surrounding def/class
for i in range(150, 230):
    if i < 0 or i >= len(lines): 
        continue
    print(f"{i+1:4d}: {lines[i]}")
PY

Repository: Project-MONAI/MONAI

Length of output: 5311


Add stacklevel=2 to the warning in LoadImage.__init__.

monai/transforms/io/array.py line 217 uses warnings.warn(...) without stacklevel, so the warning location points at MONAI internals instead of the user call site.

Suggested change
-                    warnings.warn(f"{_r} is not supported with the given parameters {args} {kwargs}.")
+                    warnings.warn(
+                        f"{_r} is not supported with the given parameters {args} {kwargs}.",
+                        stacklevel=2,
+                    )
🧰 Tools
🪛 Ruff (0.15.15)

[warning] 217-217: No explicit stacklevel keyword argument found

Set stacklevel=2

(B028)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@monai/transforms/io/array.py` at line 217, The warning call in
LoadImage.__init__ currently uses warnings.warn(f"{_r} is not supported with the
given parameters {args} {kwargs}.") which points the stack to MONAI internals;
update this call to pass stacklevel=2 so the emitted warning points to the
user's call site (i.e., change the warnings.warn call inside the
LoadImage.__init__ method to include stacklevel=2).

Source: Linters/SAST tools

self.register(the_reader())
try:
self.register(the_reader())
except OptionalImportError as e:
raise OptionalImportError(
f"required package for reader {_r} is not installed, or the version doesn't match requirement."
) from e
elif inspect.isclass(_r):
self.register(_r(*args, **kwargs))
else:
Expand Down
14 changes: 12 additions & 2 deletions tests/data/test_image_rw.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,18 @@ def test_rgb(self, reader, writer):
class TestRegRes(unittest.TestCase):
def test_0_default(self):
self.assertTrue(len(resolve_writer(".png")) > 0, "has png writer")
self.assertTrue(len(resolve_writer(".nrrd")) > 0, "has nrrd writer")
self.assertTrue(len(resolve_writer("unknown")) > 0, "has writer")
_, has_nibabel = optional_import("nibabel")
_, has_itk = optional_import("itk", allow_namespace_pkg=True)
if has_nibabel or has_itk:
self.assertTrue(len(resolve_writer(".nrrd")) > 0, "has nrrd writer")
self.assertTrue(len(resolve_writer("unknown")) > 0, "has writer")
else:
with self.assertRaises(OptionalImportError) as ctx:
resolve_writer(".nrrd")
self.assertIn("Please install: itk or nibabel.", str(ctx.exception))
with self.assertRaises(OptionalImportError) as ctx:
resolve_writer("unknown")
self.assertIn("Please install: itk or nibabel.", str(ctx.exception))
register_writer("unknown1", lambda: (_ for _ in ()).throw(OptionalImportError))
with self.assertRaises(OptionalImportError):
resolve_writer("unknown1")
Expand Down
23 changes: 21 additions & 2 deletions tests/data/test_init_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,28 @@ def test_load_image(self):
self.assertIsInstance(instance1, LoadImage)
self.assertIsInstance(instance2, LoadImage)

from monai.utils import optional_import, OptionalImportError
pkg_map = {
"NibabelReader": "nibabel",
"PILReader": "PIL",
"ITKReader": "itk",
"NrrdReader": "nrrd",
"NumpyReader": "numpy",
"PydicomReader": "pydicom",
}
for r in ["NibabelReader", "PILReader", "ITKReader", "NumpyReader", "NrrdReader", "PydicomReader", None]:
inst = LoadImaged("image", reader=r)
self.assertIsInstance(inst, LoadImaged)
if r is None:
inst = LoadImaged("image", reader=r)
self.assertIsInstance(inst, LoadImaged)
continue

_, has_pkg = optional_import(pkg_map[r])
if has_pkg:
inst = LoadImaged("image", reader=r)
self.assertIsInstance(inst, LoadImaged)
else:
with self.assertRaises(OptionalImportError):
LoadImaged("image", reader=r)
Comment on lines +47 to +53

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n --type=py 'optional_import\("itk"' -C2

Repository: Project-MONAI/MONAI

Length of output: 5327


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the exact code under review
sed -n '1,140p' tests/data/test_init_reader.py | nl -ba | sed -n '35,90p'

# Find the pkg_map definition / ITKReader mapping and the exact optional_import call site
rg -n "pkg_map|ITKReader|LoadImaged|test_load_image|optional_import" tests/data/test_init_reader.py -n

# Inspect optional_import signature/docs in repo
optional_import_file="$(rg -n --type=py "def optional_import" -S . | head -n 1 | cut -d: -f1)"
echo "Found optional_import definition in: $optional_import_file"
sed -n '1,200p' "$optional_import_file" | nl -ba | sed -n '1,200p'

Repository: Project-MONAI/MONAI

Length of output: 106


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show reviewed section with line numbers (no `nl` in this sandbox)
sed -n '1,120p' tests/data/test_init_reader.py | cat -n | sed -n '35,90p'

# Locate the exact test and the ITK probe line
rg -n "TestInitLoadImage|test_load_image|pkg_map|optional_import|ITKReader|LoadImaged" tests/data/test_init_reader.py

# Find optional_import implementation and show its signature/defaults
opt_file="$(rg -n --type=py "def optional_import" -S . | head -n 1 | cut -d: -f1)"
echo "optional_import definition: $opt_file"
sed -n '1,220p' "$opt_file" | cat -n | sed -n '1,220p'

Repository: Project-MONAI/MONAI

Length of output: 13955


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Get line number of optional_import definition
opt_def_line="$(rg -n --type=py "^def optional_import\\b" monai/utils/module.py | head -n 1 | cut -d: -f1)"
echo "optional_import def line: $opt_def_line"

# Print a window around the function start (signature + docstring header)
start=$((opt_def_line-20))
end=$((opt_def_line+120))
sed -n "${start},${end}p" monai/utils/module.py | cat -n

# Also find any occurrences of allow_namespace_pkg inside the function/file
rg -n "allow_namespace_pkg" monai/utils/module.py | head -n 50

Repository: Project-MONAI/MONAI

Length of output: 7091


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find where ITKReader is resolved/used during LoadImaged/LoadImage
rg -n "ITKReader" monai -S
rg -n "LoadImaged\\(|LoadImage\\(" monai/transforms -S
rg -n "reader=.*ITKReader|resolve_reader|resolve_reader\\(|image_reader" monai -S

# Inspect relevant reader resolution code (likely monai/data/image_reader.py and monai/data/__init__.py / transforms loader)
sed -n '1,220p' monai/data/image_reader.py | cat -n

# Also inspect image_writer if needed (for writer mapping consistency)
sed -n '1,140p' monai/data/image_writer.py | cat -n

Repository: Project-MONAI/MONAI

Length of output: 21170


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n --type=py "^def require_pkg\\b" monai/utils/module.py
req_file="monai/utils/module.py"

req_line="$(rg -n --type=py "^def require_pkg\\b" "$req_file" | head -n 1 | cut -d: -f1)"
echo "require_pkg def line: $req_line"

start=$((req_line-20))
end=$((req_line+220))
sed -n "${start},${end}p" "$req_file" | cat -n

# See whether allow_namespace_pkg is referenced inside require_pkg
rg -n "allow_namespace_pkg" "$req_file" | head -n 50

Repository: Project-MONAI/MONAI

Length of output: 10172


🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "has_itk|\\bitk\\b" monai/data/image_reader.py

Repository: Project-MONAI/MONAI

Length of output: 1442


Handle ITK as a namespace package in TestInitLoadImage.test_load_image

tests/data/test_init_reader.py:47 probes ITK with optional_import(pkg_map[r]) (defaults allow_namespace_pkg=False), but runtime monai/data/image_reader.py loads ITK with optional_import("itk", allow_namespace_pkg=True, so namespace-style ITK installs can make has_pkg false and cause a false OptionalImportError expectation.

  • Same mismatch likely applies to nrrd (runtime also uses allow_namespace_pkg=True there, but this test doesn’t special-case it).
Suggested change
-            _, has_pkg = optional_import(pkg_map[r])
+            import_kwargs = {"allow_namespace_pkg": True} if pkg_map[r] == "itk" else {}
+            _, has_pkg = optional_import(pkg_map[r], **import_kwargs)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_, has_pkg = optional_import(pkg_map[r])
if has_pkg:
inst = LoadImaged("image", reader=r)
self.assertIsInstance(inst, LoadImaged)
else:
with self.assertRaises(OptionalImportError):
LoadImaged("image", reader=r)
import_kwargs = {"allow_namespace_pkg": True} if pkg_map[r] == "itk" else {}
_, has_pkg = optional_import(pkg_map[r], **import_kwargs)
if has_pkg:
inst = LoadImaged("image", reader=r)
self.assertIsInstance(inst, LoadImaged)
else:
with self.assertRaises(OptionalImportError):
LoadImaged("image", reader=r)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/data/test_init_reader.py` around lines 47 - 53,
TestInitLoadImage.test_load_image currently calls optional_import(pkg_map[r])
(defaulting allow_namespace_pkg=False) to set has_pkg, but runtime code imports
ITK (and NRRD) with optional_import(..., allow_namespace_pkg=True), causing
false negatives; update the test to call optional_import(pkg_map[r],
allow_namespace_pkg=True) when probing readers for "itk" and "nrrd" (or set
allow_namespace_pkg=True for those pkg_map entries) so has_pkg matches the
runtime import behavior used by LoadImaged and the OptionalImportError
expectation is correct.


@SkipIfNoModule("nibabel")
@SkipIfNoModule("cupy")
Expand Down
Loading