-
Notifications
You must be signed in to change notification settings - Fork 14
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
ENH: Don't list resources if loading a known resource fails #309
Conversation
Usually importing a resource fails because the user has passed an unknown resource type, so we report the known resources in the exception message. But if the import of a known resource fails (e.g., because the resource imports an uninstalled module), it's confusing to show a list of known resources. Closes ReproNim#297.
Codecov Report
@@ Coverage Diff @@
## master #309 +/- ##
==========================================
+ Coverage 88.59% 88.63% +0.04%
==========================================
Files 133 133
Lines 9582 9635 +53
==========================================
+ Hits 8489 8540 +51
- Misses 1093 1095 +2
Continue to review full report at Codecov.
|
niceman/resource/base.py
Outdated
@@ -134,10 +134,12 @@ def factory(config): | |||
except Exception as exc: | |||
# Typically it should be an ImportError, but let's catch and recast | |||
# anything just in case. | |||
msg = exc_str(exc) | |||
known = ResourceManager._discover_types() |
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.
I wouldn't be surprised if we manage to raise exception there as well, so may be try/except these 3 lines as well and add caught exception message (msg += " Failed to discover resource types: %s" % exc_str(exc2)) in except for it?
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.
Seems like an overkill to me, but as you wish.
Thanks! |
Usually importing a resource fails because the user has passed an
unknown resource type, so we report the known resources in the
exception message. But if the import of a known resource fails (e.g.,
because the resource imports an uninstalled module), it's confusing to
show a list of known resources.
Closes #297.