Matt/icechunk encoding #2
Conversation
| @@ -130,6 +131,11 @@ async def write_virtual_variable_to_icechunk( | |||
| # TODO we should probably be doing some encoding of those attributes? | |||
| arr.attrs["DIMENSION_NAMES"] = var.dims | |||
|
|
|||
| _encoding_keys = {"_FillValue", "missing_value", "scale_factor", "add_offset"} | |||
There was a problem hiding this comment.
filters and compressors are packed in the codec pipeline, but CF encoding params are not
|
|
||
| # check chunk references | ||
| # TODO we can't explicitly check that the path/offset/length is correct because icechunk doesn't yet expose any get_virtual_refs method | ||
|
|
||
| expected_ds = open_dataset(netcdf4_file) | ||
| expected_air_array = expected_ds["air"].to_numpy() | ||
| expected_air_array = expected_ds["air"].to_numpy() / scale_factor |
There was a problem hiding this comment.
So here we are basically replicating the step that xarray would do lazily if we opened the icechunk store using xarray?
There was a problem hiding this comment.
ya... I just switched it around to be more clear
|
OK leaving this here for a short while.
All of these can be overcome i think, up to you if you want to sync this with your branch |
Is there a simple solution to this? You can't change the dtype of a
Right, annoying isn't it 😅 I don't actually think there is an upstream kerchunk issue for that - we should raise one. |
|
I would like to merge this into my branch but I'm not sure about this
Shouldn't this step become part of the codec somehow? |
|
Currently as implanted this doesn't happen. It happens when I switched to using arrays internal encoding functions. |
|
Okay so basically this problem only crops up when you try to integrate the code path here with the codepath in xarray's |
|
yupp correct |
Not ready for primetime, but first time wroking in this repo want to make sure this approach is in the right direction of what needs to be done.
I think this also has to be done for zarr writer maybe