Save InferenceData attrs
closes #2109
Description
This will save main level attrs to "__netcdf4_attrs" and also unload if present.
Checklist
- [ ] Follows official PR format
- [ ] Includes a sample plot to visually illustrate the changes (only for plot-related functions)
- [ ] New features are properly documented (with an example if appropriate)?
- [ ] Includes new or updated tests to cover the new feature
- [ ] Code style correct (follows pylint and black guidelines)
- [ ] Changes are listed in changelog
Question is, do we need update the schema OR can we do it the correct way?
edit. Still needs tests
@OriolAbril @canyon289 you know netCDF4 better than I do, what should be the correct way to save the attrs to the main file (not in group)?
Codecov Report
Merging #2131 (114ad9e) into main (0ba14fa) will increase coverage by
0.00%. The diff coverage is77.77%.
@@ Coverage Diff @@
## main #2131 +/- ##
=======================================
Coverage 90.72% 90.73%
=======================================
Files 118 118
Lines 12569 12579 +10
=======================================
+ Hits 11403 11413 +10
Misses 1166 1166
| Impacted Files | Coverage Δ | |
|---|---|---|
| arviz/data/inference_data.py | 82.91% <77.77%> (+0.22%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Is this way working? If so I assume its the correct way :D. If its working perhaps all we need is a test and were good to go
Does the attributes variable name need to be netCDF4-specific? Can't it also be saved with e.g. Zarr?
@sethaxen very good question. It should.
Also, I think the main netcdf4 format includes a possibility for attr field at the root, but did not find a good/correct way to add it. There might be a way to do this in the docs, but atleast I did not find it.
I wonder if we actually want to create the dataset at the init and then just wrap attrs to point that?
And have one variable that will contain all the groups which will then be write/load normally?
And we just add special tag into our schema about it?
Or is this someting that Datatree will handle when it is released.
Ok, still need to figure out the zarr.
LGTM, not sure why netcdf tests are failing though would have to test locally 🤔
They fail because az.load_arviz_data("centered").to_netcdf(..) fails.
Is this the object error we see even when it should not touch the compression?
They fail because az.load_arviz_data("centered").to_netcdf(..) fails.
I can confirm this fails locally for me still with 0.13.0rc1 on a new env, but works with main. I think tests will pass now, but there might still be some issues when using netcdf. Not sure there is anything we can do on our side though.
Yeah, text handling is always a bit hard.
I'm not sure if xarray has changed the defaults already to S / U dtypes, but then working with these values is more or less not great.
Maybe we could add some autotransform for object type strings if we can be certain it works correctly, but tbh it is a slippery slope...