feat: print packages to install #8240
Conversation
| 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}.") | ||
| raise OptionalImportError( |
There was a problem hiding this comment.
Could you please also include the test case in test_image_rw.py to ensure the change works as expected? Thanks.
There was a problem hiding this comment.
@KumoLiu, should it be added as a separate class?
|
Please also sign-off to resolve the DCO issue: https://github.com/Project-MONAI/MONAI/pull/8240/checks?check_run_id=33542178018 Thanks! |
| error_match = re.search(r"`(.*?)`", str(e)) | ||
| if error_match: | ||
| required_pkg.append(error_match.group(1)) |
There was a problem hiding this comment.
I suggest you modify OptionalImportError to have a member recording the name of the package that isn't present so you don't have to do a re search. This would be used here like this raise OptionalImportError(err_msg, pkg_name) in place of the original line. The other place this is raised is here where the second argument could just be None.
There was a problem hiding this comment.
@ericspod, If I understand you correctly:
- The line here should be modified to
raise OptionalImportError(err_msg, pkg_name) - In image_writer I change this line to
raise OptionalImportError(f"No ImageWriter backend found for {fmt}.", None) - Remove exception lines with re.search in image_writer.
- And to make it all works I should modify
OptionalImportErrormodule in monai/utils/module.py by adding a functionality to track package names that aren't present?
There was a problem hiding this comment.
@ericspod, If I understand you correctly:
1. The line [here](https://github.com/Project-MONAI/MONAI/blob/e73257caa79309dcce1e93abf1632f4bfd75b11f/monai/utils/module.py#L473) should be modified to `raise OptionalImportError(err_msg, pkg_name)` 2. In image_writer I change this [line](https://github.com/Project-MONAI/MONAI/blob/dev/monai/data/image_writer.py#L119) to `raise OptionalImportError(f"No ImageWriter backend found for {fmt}.", None)` 3. Remove exception lines with re.search in image_writer. 4. And to make it all works I should modify `OptionalImportError` module in monai/utils/module.py by adding a functionality to track package names that aren't present?
I think that's it, that'll allow you to get the name of the package without assumptions about the exception's error string, it'll be a more robust way of doing this going forward. Thanks!
Fixes #7980. Closes #8001.
Description
A few sentences describing the changes proposed in this pull request.
Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.