IO import issues #787
IO import issues #787
Conversation
Codecov Report
@@ Coverage Diff @@
## master #787 +/- ##
==========================================
- Coverage 95.29% 95.26% -0.03%
==========================================
Files 64 64
Lines 8944 8952 +8
==========================================
+ Hits 8523 8528 +5
- Misses 421 424 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
mtar
requested review from
Markus-Goetz and
ClaudiaComito
and removed request for
Markus-Goetz
|
I think I might have accidentally introduced this regression during the documentation overhaul. One problem remains with the current form, the conditionally defined functions, e.g. save_hdf5, are not included in the documentation. Do you see a way of fixing this? |
|
could we append the function name to |
|
also i am fine with the PR not hitting the coverage target. this a fix that simply wont unless we test it with multiple venvs on the system, which seems like overkill |
heat/core/io.py
Outdated
| if supports_hdf5(): | ||
| return load_hdf5(path, *args, **kwargs) | ||
| else: | ||
| raise ImportError("hdf5 is required for file extension {}".format(extension)) |
There was a problem hiding this comment.
i think that these should be either RunTimeErrors or TypeErrors. the issue that this is talking about is that the package is not installed.
There was a problem hiding this comment.
I was using ImportError because it's an external package that needs to be imported.
I don't know if TypeError is good here as the function gets the right argument types. ValueError maybe.
A general RuntimeError might be the best here.
…eat into bug/786-io-depends
Description
Fixes an import error in io.py when optional dependencies are not available and changes error message when saving or loading a file that requires an optional dependency.
Issue resolved: #786
Issue resolved: #697
Changes proposed:
Type of change
Due Diligence
Does this change modify the behaviour of other functions? If so, which?
no