-
Notifications
You must be signed in to change notification settings - Fork 10
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
save and reload #124
save and reload #124
Conversation
Previously it was an error to save a BoutDataset that had non-empty options. Now just warn instead - temporary workaround as now the BoutDataset can be saved, but the input file must be given again when re-loading the BoutDataset.
Codecov Report
@@ Coverage Diff @@
## master #124 +/- ##
==========================================
+ Coverage 50.29% 53.16% +2.86%
==========================================
Files 11 11
Lines 1175 1217 +42
Branches 234 246 +12
==========================================
+ Hits 591 647 +56
+ Misses 512 496 -16
- Partials 72 74 +2
Continue to review full report at Codecov.
|
To restore the metadata when reloading a saved Dataset, will need to know which attributes are 'metadata', so when converting a dict to attrs, include a prefix with the name of the dict.
New function reload_boutdataset() loads a BoutDataset from a netCDF file created by `ds.bout.save()`. Necessary because for example dimensions may be renamed when adding geometry, and metadata must be converted from dict to attributes to be saved, so reloading the saved Dataset is not the same as initial loading from BOUT++ output files.
Saving as float32 and loading still does not work.
As noted in the updated PR description at the top, this PR now updates the saving functionality, and adds |
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.
LGTM, just small change to using mutable object as a default value
Co-authored-by: Peter Hill <[email protected]>
Thanks for the review @ZedThree. Mutable default fixed now, and in a few extra places too! |
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.
This looks good, only very minor comments
dict_to_attrs(to_save, 'metadata') | ||
# Must do this for all variables and coordinates in dataset too | ||
for varname, da in chain(to_save.data_vars.items(), to_save.coords.items()): | ||
try: | ||
dict_to_attrs(da, key='metadata') | ||
dict_to_attrs(da, 'metadata') | ||
except KeyError: |
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.
What would happen if there isn't any metadata?
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.
If there's no metadata on a variable then there will be a KeyError
and that nothing is done for that variable.
Tests are now failing for the cases that use the latest versions of installed packages. I tried upgrading to the latest released |
Was fixed, just not by merging the suggested change (some other lines needed changing too).
Changes to allow a BoutDataset to be saved to a file and re-loaded, replicating the state of the original BoutDataset.
Activates previously-skipped tests of
save()
as well as adding tests usingreload_boutdataset()
.Needs to deal with
options
. Previously it was an error to save aBoutDataset
that had non-emptyoptions
. Now just warn instead - temporary workaround (until #94, #97 are resolved) as now theBoutDataset
can be saved, but the input file must be given again when re-loading theBoutDataset
.