Zarr ZipStore versus DirectoryStore: ZipStore requires .close()

[Huite]

I was saving my dataset into a ZipStore -- apparently succesfully -- but then I couldn't reopen them.

The issue appears to be that a regular DirectoryStore behaves a little differently: it doesn't need to be closed, while a ZipStore.

(I'm not sure how this relates to #2586, the remarks there don't appear to be applicable anymore.)

MCVE Code Sample

This errors:

import xarray as xr
import zarr

# works as expected 
ds = xr.Dataset({'foo': [2,3,4], 'bar': ('x', [1, 2]), 'baz': 3.14})
ds.to_zarr(zarr.DirectoryStore("test.zarr"))
print(xr.open_zarr(zarr.DirectoryStore("test.zarr")))

# error with ValueError "group not found at path ''
ds.to_zarr(zarr.ZipStore("test.zip"))
print(xr.open_zarr(zarr.ZipStore("test.zip")))

Calling close, or using with does the trick:

store = zarr.ZipStore("test2.zip")
ds.to_zarr(store)
store.close()
print(xr.open_zarr(zarr.ZipStore("test2.zip")))

with zarr.ZipStore("test3.zip") as store:
     ds.to_zarr(store)
print(xr.open_zarr(zarr.ZipStore("test3.zip")))

Expected Output

I think it would be preferable to close the ZipStore in this case. But I might be missing something?

Problem Description

Because to_zarr works in this situation with a DirectoryStore, it's easy to assume a ZipStore will work similarly. However, I couldn't get it to read my data back in this case.

Versions

Output of xr.show_versions()

INSTALLED VERSIONS

commit: None
python: 3.7.6 | packaged by conda-forge | (default, Jan 7 2020, 21:48:41) [MSC v.1916 64 bit (AMD64)]
python-bits: 64
OS: Windows
OS-release: 10
machine: AMD64
processor: Intel64 Family 6 Model 158 Stepping 9, GenuineIntel
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: None.None
libhdf5: 1.10.5
libnetcdf: 4.7.3

xarray: 0.15.2.dev41+g8415eefa.d20200419
pandas: 0.25.3
numpy: 1.17.5
scipy: 1.3.1
netCDF4: 1.5.3
pydap: None
h5netcdf: None
h5py: 2.10.0
Nio: None
zarr: 2.4.0
cftime: 1.0.4.2
nc_time_axis: None
PseudoNetCDF: None
rasterio: 1.1.2
cfgrib: None
iris: None
bottleneck: 1.3.2
dask: 2.14.0+23.gbea4c9a2
distributed: 2.14.0
matplotlib: 3.1.2
cartopy: None
seaborn: 0.10.0
numbagg: None
pint: None
setuptools: 46.1.3.post20200325
pip: 20.0.2
conda: None
pytest: 5.3.4
IPython: 7.13.0

[shoyer]

I think it would be preferable to close the ZipStore in this case. But I might be missing something?

If we make users explicitly open their own Zarr stores, then I think it's best to let them close them, themselves. If we closed them automatically, then they couldn't add their own files into the store outside of xarray.

It would make sense to automatically close stores if they were opened inside to_zarr(), e.g., if we had some logic that automatically opened a zip store when the file path ends in ".zip". This would not be a bad idea (small files are quite wasteful on most filesystems), but is not currently the case, as I understand it.

At the very least, we could add a note to the documentation about this.

[Huite]

Opening (and closing) a ZipStore when given a path ending in ".zip" would largely solve it I believe -- in fact, I tried just providing a path with ".zip" to to_zarr before, hoping it would work. (And indeed, I did so because the many small files are troublesome.)

open_zarr already automatically accepts path ".zip", so it would be nicely symmetric as well.
However, this behavior exists within Zarr, not Xarray.

https://github.com/zarr-developers/zarr-python/blob/473576f3b17ac4a460dfccbcc4860606cd8123ed/zarr/creation.py#L130-142

I'd be happy to have a try at a PR in principle.

Maybe it could be as simple as adding a with?

if isinstance(store, (str, pathlib.Path)):
    if str(store).endswith('.zip'):
        with zarr.ZipStore(store) as zstore:
# etc

In the xarray.backends.api.to_zarr() function.

[shoyer]

open_zarr already automatically accepts path ".zip", so it would be nicely symmetric as well.
However, this behavior exists within Zarr, not Xarray.

Yes, this would be nice to copy in xarray. I suspect we could implement this in very similar way to what you suggest here.

[stale]

In order to maintain a list of currently relevant issues, we mark issues as stale after a period of inactivity

If this issue remains relevant, please comment here or remove the stale label; otherwise it will be marked as closed automatically