Fix duplicate appending of scalars in HDF writer and add an overwrite option #1366
Conversation
jaladh-singhal
requested review from
wkerzendorf and
andrewfullard
and removed request for
wkerzendorf
Codecov Report
@@ Coverage Diff @@
## master #1366 +/- ##
==========================================
- Coverage 71.77% 71.72% -0.06%
==========================================
Files 66 66
Lines 5077 5071 -6
==========================================
- Hits 3644 3637 -7
- Misses 1433 1434 +1
Continue to review full report at Codecov.
|
tardis/io/util.py
Outdated
| Returns | ||
| ------- | ||
| overwrite: bool | ||
| If the HDF file path already exists, whether overwrite it or not |
There was a problem hiding this comment.
If the HDF file path already exists, whether to overwrite it or not
tardis/io/util.py
Outdated
| `overwrite` option doesn't have any effect when `path_or_buf` is an | ||
| HDFStore because it depends on user that in which mode they've opened | ||
| it. |
There was a problem hiding this comment.
overwrite option doesn't have an effect when path_or_buf is an HDFStore because the user decides on the mode in which they have opened the HDF (path or HDFStore).
There was a problem hiding this comment.
Umm...actually I meant, file opening mode ('r', 'w', 'a') - when specifying HDFStore, user has control over the mode in their hands, so overwrite option won't have any effect.
There was a problem hiding this comment.
Rephrased it - let me know if it still needs changes!
Fix duplicate appending of scalars in HDF writer and add an overwrite option
When saving a simulation object in HDF using
to_hdf()to same file path, the scalars are appended instead of being overwritten causing duplicate values in scalrs series of HDF. This is unlike what happens to other properties like numpy arrays, dataframes, etc. which are always overwritten.Besides making scalars overwrite (instead of append), this PR makes sure to explicitly prompt users when overwriting a pre-existing HDF file (which earlier used to happen implicitly). For this, a boolean
overwriteoption is added toto_hdf()which is by defaultFalse.Changes made
tardis/io/util.py- fixed scalars appending, added overwrite option & improved related docstringsto_hdf()- addedoverwrite=Truesince parameterization and session scoped fixtures use same hdf file path and earlier implementation implicitly overwrote the written HDFs.How Has This Been Tested?
Local tests pass, hopefully Azure pipelines pass too!