Skip to content

Commit

Permalink
some nits after merging w/ main
Browse files Browse the repository at this point in the history
norlandrhagen committed Jan 9, 2025
1 parent 9e44a8a commit 87dbdae
Showing 5 changed files with 20 additions and 17 deletions.
6 changes: 4 additions & 2 deletions ci/upstream.yml
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@ channels:
- conda-forge
- nodefaults
dependencies:
- xarray>=2024.10.0
- h5netcdf
- h5py
- hdf5
@@ -27,8 +26,11 @@ dependencies:
- pooch
- fsspec
- dask
- zarr>=3.0.0
- pip
- pip:
- icechunk>=0.1.0a8 # Installs zarr v3 as dependency
- xarray>=2025.1.1

- icechunk>=0.1.0a9 # Installs zarr v3 as dependency
# - git+https://github.com/fsspec/kerchunk@main # kerchunk is currently incompatible with zarr-python v3 (https://github.com/fsspec/kerchunk/pull/516)
- imagecodecs-numcodecs==2024.6.1
2 changes: 1 addition & 1 deletion virtualizarr/manifests/manifest.py
Original file line number Diff line number Diff line change
@@ -96,7 +96,7 @@ def validate_and_normalize_path_to_uri(path: str, fs_root: str | None = None) ->
# using PosixPath here ensures a clear error would be thrown on windows (whose paths and platform are not officially supported)
_path = PosixPath(path)

if not _path.suffix:
if not _path.suffix and "zarr" not in path:
raise ValueError(
f"entries in the manifest must be paths to files, but this path has no file suffix: {path}"
)
2 changes: 1 addition & 1 deletion virtualizarr/readers/common.py
Original file line number Diff line number Diff line change
@@ -51,7 +51,7 @@ def maybe_open_loadable_vars_and_indexes(

if fpath.upath.suffix == ".zarr":
engine = "zarr"
xr_input = fpath.filepath
xr_input = fpath.upath

else:
engine = None
8 changes: 3 additions & 5 deletions virtualizarr/readers/zarr.py
Original file line number Diff line number Diff line change
@@ -94,7 +94,6 @@ async def build_chunk_manifest(
"""Build a ChunkManifest with the from_arrays method"""
import numpy as np

# import pdb; pdb.set_trace()
key_tuples = [(x,) async for x in zarr_array.store.list_prefix(prefix)]

filepath_list = [filepath] * len(key_tuples)
@@ -281,13 +280,12 @@ async def _open_virtual_dataset(
loadable_variables,
)

filepath = validate_and_normalize_path_to_uri(
filepath, fs_root=Path.cwd().as_uri()
)
# filepath = validate_and_normalize_path_to_uri(
# filepath, fs_root=Path.cwd().as_uri()
# )
# This currently fails for local filepaths (ie. tests) but works for s3:
# *** TypeError: Filesystem needs to support async operations.
# https://github.com/zarr-developers/zarr-python/issues/2554

if reader_options is None:
reader_options = {}

19 changes: 11 additions & 8 deletions virtualizarr/tests/test_readers/test_zarr.py
Original file line number Diff line number Diff line change
@@ -4,27 +4,30 @@

from virtualizarr import open_virtual_dataset
from virtualizarr.manifests import ManifestArray
from virtualizarr.tests import network, requires_zarrV3
from virtualizarr.tests import requires_network, requires_zarrV3

# It seems like this PR: https://github.com/zarr-developers/zarr-python/pull/2533
# might fix this issue: https://github.com/zarr-developers/zarr-python/issues/2554


@requires_zarrV3
@network
@requires_network
@pytest.mark.parametrize(
"zarr_store",
[
pytest.param(
2,
id="Zarr V2",
marks=pytest.mark.xfail(
reason="https://github.com/zarr-developers/zarr-python/issues/2554"
),
# marks=pytest.mark.xfail(
# reason="https://github.com/zarr-developers/zarr-python/issues/2554"
# ),
),
pytest.param(
3,
id="Zarr V3",
marks=pytest.mark.xfail(
reason="https://github.com/zarr-developers/zarr-python/issues/2554"
),
# marks=pytest.mark.xfail(
# reason="https://github.com/zarr-developers/zarr-python/issues/2554"
# ),
),
],
indirect=True,

0 comments on commit 87dbdae

Please sign in to comment.