-
-
Notifications
You must be signed in to change notification settings - Fork 393
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 InferenceData attrs #2131
Save InferenceData attrs #2131
Conversation
@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
@@ Coverage Diff @@
## main #2131 +/- ##
=======================================
Coverage 90.72% 90.73%
=======================================
Files 118 118
Lines 12569 12579 +10
=======================================
+ Hits 11403 11413 +10
Misses 1166 1166
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. |
1c706fb
to
e2ae758
Compare
LGTM, not sure why netcdf tests are failing though would have to test locally 🤔 |
They fail because Is this the object error we see even when it should not touch the compression? |
e2ae758
to
114ad9e
Compare
I can confirm this fails locally for me still with |
Yeah, text handling is always a bit hard. I'm not sure if xarray has changed the defaults already to 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... |
closes #2109
Description
Save attrs to root level
Checklist