Closed TomNicholas closed 2 months ago
@jsignell summoning you in case you have any thoughts / ideas here
@thodson-usgs got a similar looking error in https://github.com/zarr-developers/VirtualiZarr/pull/203#issue-2436462556, but only on more recent versions of virtualizarr. There must be some kind of regression, which we should narrow down using git bisect
.
I am taking a look. Are you sure you got the same error when you dropped the time component? I am seeing an s3 access issue when I do that (which I am taking to mean I made it passed the original error).
from virtualizarr import open_virtual_dataset
vds = open_virtual_dataset(
's3://wrf-se-ak-ar5/ccsm/rcp85/daily/2060/WRFDS_2060-01-01.nc',
indexes={},
drop_variables=["Time"]
)
vds.virtualize.to_kerchunk("combined_no_t.json", format="json")
ds = xr.open_dataset('combined_no_t.json', engine="kerchunk")
btw, git bisect
led me to 10bd53dc3dae08303e57fe5aefe49804d9c4517d. Maybe I can find the pre-squash branch and dig further tomorrow.
Here's the bug: https://github.com/zarr-developers/VirtualiZarr/blob/179bb2ab42664546dd243a2e04fab8737846229a/virtualizarr/zarr.py#L70
Reverting this line back to https://github.com/zarr-developers/VirtualiZarr/blob/0ad4de5c612d1d632c2acb07ecfad071756eccf4/virtualizarr/zarr.py#L47 causes my test to pass.
I propose changing this to
fill_value: FillValueT = Field(default=np.nan, validate_default=True)
which also passes.
AFAICT, 0.0
is the appropriate default fill value. That matches what zarr-python does. The line raising an exception is I think something like
In [28]: np.array([0.0], dtype=np.dtype("datetime64[ns]"))
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
Cell In[28], line 1
----> 1 np.array([0.0], dtype=np.dtype("datetime64[ns]"))
Called via
zarr.v2.meta.Metadata2.decode_fill_value(np.nan, np.dtype("datetime64[ns]"))
But that line fails with a fill value of np.nan
and 0.0
. @thodson-usgs would you be to get a debugger in there and see what the values of flil_value
and dtype
are both before and after https://github.com/zarr-developers/VirtualiZarr/commit/10bd53dc3dae08303e57fe5aefe49804d9c4517d? Or share a file somewhere public so I can take a look?
Thanks @TomAugspurger, I put an example back on #206. These might indeed be the same issue, but I want to be careful about crossing streams here.
I'm trying to debug @thodson-usgs's example from https://github.com/cubed-dev/cubed/pull/520 (and originally https://github.com/zarr-developers/VirtualiZarr/pull/197).
He is doing a whole serverless reduction of virtual references to multiple files (!!! - relevant to #123), but there seem to be some more basic errors to be fixed first.
Specifically, if I try to use virtualizarr on just one of his files this happens:
At first I assumed there was something wrong with our handling of the loaded
cftime_variables
, but actually even if I drop the'Time'
variable I still get exactly the same error:I don't know why it's even trying to convert anything to a datetime - none of the other variables have units of time.
What's also weird is that this is raised from within
meta.py:260, in Metadata2.decode_fill_value(cls, v, dtype, object_codec)
, which suggests a problem with thefill_value
. But I checked and all of the variables in this virtual dataset have a fill_value of either a float ornan
in their.encoding
, again nothing about a datetime.