4456 optional import issue on top-level modules -- monai.handlers/monai.engines

[wyli]

Fixes #4456

Description

make sure that if the top-level modules miss dependencies, the optional import error raises properly.

Types of changes

• 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.

[wyli]

hi @ericspod could you please help review this PR? this is to correct the error message when top-level modules (monai.engines and monai.handlers) are not imported by load_modules, it now raises an optional import error instead of an attribute error. All integration tests work fine.

[ericspod]

The way of loading modules needs to be reconsidered a bit here since the two stage process was from some very old code of mine. We aren't using the @export decorator which it was intended for. The immediate issue here however is that Metric object here raises the exception when it is used as a parent class, I would say it should raise the exception when one tries to instantiate it instead. Because the exception is raised the module doesn't get created, your solution here doesn't raise the error but will return a LazyRaise object which is a bit confusing as well. Could we change the behaviour of LazyRaise so that monai.handlers is an actual module with classes which raises exceptions on instantiation?

[wyli]

thanks @ericspod, it seems it's possible to achieve what you suggested. I've added an as_type bool to optional_import, when it's true, it returns the class instead of an instance so other types can inherit. I've updated the PR, please let me know what you think...

[ericspod]

The code from issue #4456 now works in that it gives me a module object back as expected, however it permits me to instantiate monai.handlers.IgniteMetric without Ignite being installed, it does raise an exception when I try to call any of its methods. If that's expected then it works and I'm good with the changes, I would still suggest some tweak that will raise the exception on instantiation however. This may require a different class from LazyRaise which has a constructor accepting any arguments and doing nothing but raising the exception.

[wyli]

updated to raise an error when calling __init__, the only corner case is the decorator usage:

MONAI/monai/handlers/ignite_metric.py

Line 63 in d64c6d2

@reinit__is_reduced

it'll also call the constructor, so I add some check for this case. @ericspod

[ericspod]

Works for me as expected now, thanks!

[wyli]

/build

[wyli]

/build