New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expanded import_or_raise
to catch all exceptions
#759
Conversation
Codecov Report
@@ Coverage Diff @@
## master #759 +/- ##
=======================================
Coverage 99.38% 99.38%
=======================================
Files 151 151
Lines 5532 5545 +13
=======================================
+ Hits 5498 5511 +13
Misses 34 34
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just left a small suggestion to use mocking + a minor typo, but 👍
@@ -20,6 +20,9 @@ def import_or_raise(library, error_msg=None): | |||
error_msg = "" | |||
msg = (f"Missing optional dependency '{library}'. Please use pip to install {library}. {error_msg}") | |||
raise ImportError(msg) | |||
except Exception as ex: | |||
msg = (f"An exception occured while trying to import `{library}`: {str(ex)}") | |||
raise Exception(msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. This will be a great place to add some logging once @angela97lin's work adds it!
0d2ac84
to
1a8dba2
Compare
1a8dba2
to
8c83915
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! LGTM 👍
Resolves #729