Fix issue 7980 #8671
Fix issue 7980 #8671
Conversation
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit. Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit. Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: 7ce29a1 Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: cc0c951 Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: 7ce29a1 Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: cc0c951 Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: 3e92d74 Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
DCO Remediation Commit for Mohammad Amanour Rahman <amanourrahman@gmail.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to the previous commits.
aman0311x
requested review from
KumoLiu,
Nic-Ma,
ericspod and
virginiafdez
as code owners
📝 WalkthroughWalkthroughEnhanced error messaging in monai/data/image_writer.py: when no ImageWriter backends are available for an extension, resolve_writer now adds a RECOMMENDED_PACKAGES mapping and appends an install hint to the raised OptionalImportError. In monai/losses/perceptual.py the enum name Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/data/image_writer.pymonai/losses/perceptual.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. 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. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/losses/perceptual.pymonai/data/image_writer.py
🧬 Code graph analysis (1)
monai/data/image_writer.py (1)
monai/utils/module.py (1)
OptionalImportError(309-312)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: build-docs
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: packaging
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-os (macOS-latest)
🔇 Additional comments (3)
monai/data/image_writer.py (1)
133-137: Enhanced error message looks good.The error message construction is clear and actionable, providing users with specific installation guidance.
monai/losses/perceptual.py (2)
79-79: Enum usage updates are consistent.All references to the corrected
PerceptualNetworkTypeenum are properly updated in the default parameter (line 79), validation logic (line 102), and error message (line 105).Also applies to: 102-102, 105-105
26-33: Fix docstring reference to use correct enum name.The actual
__init__signature uses the correct enum namePerceptualNetworkType(line 80), but the docstring at line 52 still references the old misspelled namePercetualNetworkType. Update the docstring to match the actual code. This is not a breaking API change since the enum itself is correctly defined and only the documentation needs correction.
monai/data/image_writer.py
Outdated
| RECOMMENDED_PACKAGES = { | ||
| "png": "Pillow", | ||
| "jpg": "Pillow", | ||
| "jpeg": "Pillow", | ||
| "nii": "nibabel or SimpleITK", | ||
| "nii.gz": "nibabel or SimpleITK", | ||
| "nrrd": "pynrrd", | ||
| "tif": "Pillow or tifffile", | ||
| "tiff": "Pillow or tifffile", | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move RECOMMENDED_PACKAGES to module level.
Defining this dictionary inside the error handling block causes it to be recreated on every error. Define it as a module-level constant instead.
🔎 Proposed refactor
Move the dictionary definition after line 64 (after SUPPORTED_WRITERS):
SUPPORTED_WRITERS: dict = {}
+
+RECOMMENDED_PACKAGES = {
+ "png": "Pillow",
+ "jpg": "Pillow",
+ "jpeg": "Pillow",
+ "nii": "nibabel or SimpleITK",
+ "nii.gz": "nibabel or SimpleITK",
+ "nrrd": "pynrrd",
+ "tif": "Pillow or tifffile",
+ "tiff": "Pillow or tifffile",
+}Then at line 119, remove the local definition:
if not avail_writers and error_if_not_found:
- RECOMMENDED_PACKAGES = {
- "png": "Pillow",
- "jpg": "Pillow",
- "jpeg": "Pillow",
- "nii": "nibabel or SimpleITK",
- "nii.gz": "nibabel or SimpleITK",
- "nrrd": "pynrrd",
- "tif": "Pillow or tifffile",
- "tiff": "Pillow or tifffile",
- }
-
- fmt_clean = fmt.replace(".", "").lower()
- package_hint = RECOMMENDED_PACKAGES.get(fmt_clean, "")
+ package_hint = RECOMMENDED_PACKAGES.get(fmt, "")🤖 Prompt for AI Agents
In monai/data/image_writer.py around lines 119 to 128, RECOMMENDED_PACKAGES is
currently defined inside an exception/error handling block causing it to be
recreated on every error; move this dictionary to module level (e.g., place it
after SUPPORTED_WRITERS around line 64) as a top-level constant and remove the
local definition at line 119 so the code references the module-level constant
instead.
monai/data/image_writer.py
Outdated
| fmt_clean = fmt.replace(".", "").lower() | ||
| package_hint = RECOMMENDED_PACKAGES.get(fmt_clean, "") |
There was a problem hiding this comment.
Fix format lookup bug for multi-dot extensions.
Line 130 converts "nii.gz" to "niigz" (removes all dots), but line 124 defines the dictionary key as "nii.gz". The lookup at line 131 will always fail for "nii.gz" files. Additionally, .lower() is redundant since fmt is already lowercase from line 105.
🔎 Proposed fix
- fmt_clean = fmt.replace(".", "").lower()
- package_hint = RECOMMENDED_PACKAGES.get(fmt_clean, "")
+ package_hint = RECOMMENDED_PACKAGES.get(fmt, "")🤖 Prompt for AI Agents
In monai/data/image_writer.py around lines 130-131, the code currently
normalizes the format by removing dots and lowercasing (fmt_clean =
fmt.replace(".", "").lower()), which breaks lookup for multi-dot extensions like
"nii.gz" (dictionary key is "nii.gz") and lowercasing is redundant. Change the
normalization to preserve dots and avoid the unnecessary .lower(): set fmt_clean
= fmt (or at most fmt.strip() to remove surrounding whitespace) so
RECOMMENDED_PACKAGES.get(fmt_clean, "") can correctly find keys such as
"nii.gz".
…il.com> I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: 7ce29a1 I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: cc0c951 I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: 3e92d74 I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: f563b89 I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: a234edf I, Mohammad Amanour Rahman <amanourrahman@gmail.com>, hereby add my Signed-off-by to this commit: 2d423b8 Signed-off-by: Mohammad Amanour Rahman <amanourrahman@gmail.com>
aman0311x
force-pushed
the
fix-issue-7980
branch
from
05922f3 to
55b8430
Compare
for more information, see https://pre-commit.ci
1 participant
Fixes # .
Description
This pull request enhances the error message when a suitable image writer backend is missing for a specific file format. Currently, MONAI raises an OptionalImportError without suggesting how to fix it.
Key changes include:
Added a RECOMMENDED_PACKAGES mapping for common image formats like .png, .nii, .jpg, .nrrd, and .tiff.
Updated the error message in monai/data/image_writer.py to provide actionable guidance, suggesting the specific package the user needs to install (e.g., "Please install 'Pillow' (e.g., pip install Pillow)").
Improved the developer and user experience by making dependency-related errors easier to debug.
Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.