Change nan default fill_value for kerchunk arrays - Githubissues

zarr-developers / VirtualiZarr

Create virtual Zarr stores from archival data files using xarray syntax

https://virtualizarr.readthedocs.io/en/stable/api.html

Apache License 2.0

124 stars 24 forks source link

Change nan default fill_value for kerchunk arrays #255

Open mpiannucci opened 1 month ago

mpiannucci commented 1 month ago

This removes adding nan as the fill_value for kerchunk references that incklude fill_value=null. This is necessary because only floats can have nan values represented

[ ] Closes #xxxx
[x] Tests added
[x] Tests passing
[x] Full type hint coverage
[ ] Changes are documented in docs/releases.rst
[ ] New functions/methods are listed in api.rst
[ ] New functionality has documentation

TomNicholas commented 1 month ago

Would you mind adding a regression test? This was specifically spotted in the context of integer dtypes right?

Test failures are unrelated https://github.com/zarr-developers/VirtualiZarr/issues/249

mpiannucci commented 1 month ago

Yeah. This weekend I'll handle if it's floats and add tests for that specifically

TomNicholas commented 1 month ago

Yeah. This weekend I'll handle if it's floats and add tests for that specifically

Okay so I guess we'll wait for the tests to merge this then.

mpiannucci commented 1 month ago

Lemme know if those tests are sufficient

TomNicholas commented 1 month ago

The tests look good but something is failing