MONAI icon indicating copy to clipboard operation
MONAI copied to clipboard

Print what package should be installed when suitable writer is missing

Open ytl0623 opened this issue 1 year ago • 3 comments

Fixes #7980

Description

To implement these changes:

  1. Add the from typing import Dict, List import.
  2. Add the WRITER_PACKAGE_MAP dictionary after the existing imports.
  3. Replace the existing OptionalImportError class with the new version provided above.

For example, if you try to save a PNG file without Pillow installed, you'll now get an error message like:

# Before
monai.utils.module.OptionalImportError: No ImageWriter backend found for png.

# Now
monai.utils.module.OptionalImportError: No ImageWriter backend found for png. Please install one of the following packages: pillow

Types of changes

  • [x] Non-breaking change (fix or new feature that would not break existing functionality).
  • [ ] Breaking change (fix or new feature that would cause existing functionality to change).
  • [ ] New tests added to cover the changes.
  • [ ] Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • [ ] Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • [ ] In-line docstrings updated.
  • [ ] Documentation updated, tested make html command in the docs/ folder.

ytl0623 avatar Aug 07 '24 07:08 ytl0623

Hi @ytl0623, thanks for the pr!

Given that we already have a resolve_writer function to determine the appropriate writer based on the suffix, do you think it would be feasible to extend this function by adding a required_pkg list which can also resolve @dzenanz's concern here.

def resolve_writer():
    ...
    requried_pkg = []
        default_writers = SUPPORTED_WRITERS.get(EXT_WILDCARD, ())
        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 as e:
                requried_pkg.append(re.search(r'`(.*?)`', str(e)).group(1))
                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}, install one of the required packages in {requried_pkg}.")
    ...

KumoLiu avatar Aug 15 '24 07:08 KumoLiu

Hi @ytl0623, Do you plan to address the comments on the PR? If not, I’d be happy to assist with it. I’m hoping we can get this merged into version 1.4, but we only have about one or two weeks left to add new features.

KumoLiu avatar Aug 26 '24 05:08 KumoLiu

Hi @KumoLiu, I apologize for the delay in responding. I’m willing to assist it. It might take me a little time, but I’ll make sure to work on it and keep you updated.

ytl0623 avatar Aug 26 '24 08:08 ytl0623

@KumoLiu, hello, I want to help with merging this PR. How can I do so? Make a new PR with changes you requested?

vectorvp avatar Nov 22 '24 12:11 vectorvp

@KumoLiu, hello, I want to help with merging this PR. How can I do so? Make a new PR with changes you requested?

Sure. @ytl0623, if you’re short on time to make the update, perhaps @vectorvp could assist with it. Thanks!

KumoLiu avatar Nov 22 '24 13:11 KumoLiu

@KumoLiu, do we want to keep modifications in module.py?

vectorvp avatar Nov 26 '24 11:11 vectorvp

resolve_writer

No, we can just extend the existing resolve_writer as well.

KumoLiu avatar Nov 26 '24 12:11 KumoLiu