diff --git a/CHANGELOG.md b/CHANGELOG.md index fca29ae..ba0f3cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,16 @@ # Changelog All notable changes to this project will be documented in this file. + +## [v2607.0.0] +### Fixed +- `decode_cf`, `cache` and `create_default_indexes` were silently ignored +- `backend_kwargs={"decode_cf": False}` disagreed with `decode_cf=False` on GeoTIFFs +- `backend_kwargs` is now flattened before engine-specific sanitizing +- `decode_times` and `decode_timedelta` no longer dropped for rasterio since rioxarray accepts them +### Changed +- `open_posix()`, `open_cloud()` and `PrismBackendEntrypoint.open_dataset()` now return a raw backend dataset + ## [v2605.0.0] ## Fixed - `FileNotFoundError` for missing local files instead of misleading "cannot detect format" diff --git a/src/xarray_prism/_delegate.py b/src/xarray_prism/_delegate.py new file mode 100644 index 0000000..9524c78 --- /dev/null +++ b/src/xarray_prism/_delegate.py @@ -0,0 +1,98 @@ +""" +Delegation to the concrete xarray backend entrypoint. +""" + +from __future__ import annotations + +from typing import Any, Dict, Optional + +# Decoder flags that xarray.open_dataset resolves from decode_cf +DECODER_PARAMETERS = ( + "mask_and_scale", + "decode_times", + "decode_timedelta", + "concat_characters", + "use_cftime", + "decode_coords", +) + + +# Arguments xarray.open_dataset handles itself, after the backend +# has returned +API_ONLY_KWARGS = frozenset( + { + "cache", + "chunks", + "create_default_indexes", + "inline_array", + "chunked_array_type", + "from_array_kwargs", + "engine", + } +) + + +def _reject_api_only_kwargs(kwargs: Dict[str, Any]) -> None: + misplaced = sorted(API_ONLY_KWARGS & kwargs.keys()) + if not misplaced: + return + names = ", ".join(repr(m) for m in misplaced) + example = misplaced[0] + raise TypeError( + f"xarray-prism: {names} must be passed directly to xarray.open_dataset, " + f"not inside backend_kwargs. xarray applies these itself after the " + f"backend returns, so nesting them cannot work with any engine.\n" + f" use: xr.open_dataset(uri, engine='prism', {example}=...)\n" + f" not: xr.open_dataset(uri, engine='prism', " + f"backend_kwargs={{{example!r}: ...}})" + ) + + +def _expand_decode_cf(kwargs: Dict[str, Any]) -> Dict[str, Any]: + """ + Handle a decode_cf that reached us through backend_kwargs + """ + if "decode_cf" not in kwargs: + return kwargs + + decode_cf = kwargs.pop("decode_cf") + if decode_cf is False: + for name in DECODER_PARAMETERS: + kwargs[name] = False + return kwargs + + +def normalize_open_kwargs( + kwargs: Dict[str, Any], + backend_kwargs: Optional[Dict[str, Any]] = None, +) -> Dict[str, Any]: + """ + Flatten backend_kwargs and expand decode_cf into decoder flags + """ + merged = dict(kwargs) + if backend_kwargs: + merged.update(backend_kwargs) + _reject_api_only_kwargs(merged) + return _expand_decode_cf(merged) + + +def open_with_engine( + target: Any, + engine: str, + drop_variables: Optional[Any] = None, + backend_kwargs: Optional[Dict[str, Any]] = None, + **kwargs: Any, +) -> Any: + """ + Open target with engine's backend entrypoint + """ + from xarray.backends.plugins import get_backend + + kwargs = normalize_open_kwargs(kwargs, backend_kwargs) + + kwargs = { + k: v for k, v in kwargs.items() if not (k in DECODER_PARAMETERS and v is None) + } + + backend = get_backend(engine) + return backend.open_dataset(target, drop_variables=drop_variables, **kwargs) diff --git a/src/xarray_prism/_version.py b/src/xarray_prism/_version.py index fc9d358..8000335 100644 --- a/src/xarray_prism/_version.py +++ b/src/xarray_prism/_version.py @@ -1,3 +1,3 @@ """Version information for xarray-prism.""" -__version__ = "2605.0.0" +__version__ = "2607.0.0" diff --git a/src/xarray_prism/backends/cloud.py b/src/xarray_prism/backends/cloud.py index c0838df..e5ee29c 100644 --- a/src/xarray_prism/backends/cloud.py +++ b/src/xarray_prism/backends/cloud.py @@ -19,8 +19,7 @@ def open_cloud( **kwargs, ) -> Any: """Open remote file with detected engine.""" - import xarray as xr - + from .._delegate import open_with_engine from ..utils import gdal_env storage_options = kwargs.pop("storage_options", None) @@ -30,7 +29,7 @@ def open_cloud( # GRIB / NetCDF3: must download the full file first if engine in ("cfgrib", "scipy"): local_path = cache_remote_file(uri, engine, storage_options, show_progress) - return xr.open_dataset( + return open_with_engine( local_path, engine=engine, drop_variables=drop_variables, @@ -40,7 +39,7 @@ def open_cloud( # NetCDF4 (OPeNDAP) if engine == "netcdf4": - return xr.open_dataset( + return open_with_engine( uri, engine=engine, drop_variables=drop_variables, @@ -53,7 +52,7 @@ def open_cloud( from ..utils import sanitize_rasterio_kwargs with gdal_env(storage_options): - return xr.open_dataset( + return open_with_engine( uri, engine=engine, drop_variables=drop_variables, @@ -62,7 +61,7 @@ def open_cloud( ) # Zarr, h5netcdf - ds = xr.open_dataset( + ds = open_with_engine( uri, engine=engine, drop_variables=drop_variables, diff --git a/src/xarray_prism/backends/posix.py b/src/xarray_prism/backends/posix.py index b929a11..b3fe7b8 100644 --- a/src/xarray_prism/backends/posix.py +++ b/src/xarray_prism/backends/posix.py @@ -15,7 +15,7 @@ def open_posix( **kwargs, ) -> Any: """Open local file with detected engine.""" - import xarray as xr + from .._delegate import open_with_engine # the following posix backends don't accept storage_options _NO_STORAGE_OPTIONS = frozenset({"cfgrib", "scipy", "netcdf4", "rasterio"}) @@ -35,7 +35,7 @@ def open_posix( from ..utils import sanitize_rasterio_kwargs kwargs = sanitize_rasterio_kwargs(kwargs) - ds = xr.open_dataset( + ds = open_with_engine( uri, engine=engine, drop_variables=drop_variables, diff --git a/src/xarray_prism/entrypoint.py b/src/xarray_prism/entrypoint.py index d9e942c..0783ce8 100644 --- a/src/xarray_prism/entrypoint.py +++ b/src/xarray_prism/entrypoint.py @@ -76,7 +76,19 @@ class PrismBackendEntrypoint(BackendEntrypoint): ) url = "https://github.com/freva-org/xarray-prism" - open_dataset_parameters = ("filename_or_obj", "drop_variables", "storage_options") + + # Everything Prism forwards to the underlying engine + open_dataset_parameters = ( + "filename_or_obj", + "drop_variables", + "storage_options", + "mask_and_scale", + "decode_times", + "decode_timedelta", + "concat_characters", + "use_cftime", + "decode_coords", + ) ENGINE_MAP: Dict[str, str] = { "zarr": "zarr", @@ -154,6 +166,13 @@ def open_dataset( kwargs.pop("xarray_engine", None) backend_kwargs = kwargs.pop("backend_kwargs", None) or {} + # flatten backend_kwargs and expand a decode_cf that arrived through + # it + from ._delegate import normalize_open_kwargs + + kwargs = normalize_open_kwargs(kwargs, backend_kwargs) + backend_kwargs = {} + # Check custom registry first (handles custom uri_types too) custom_handler = registry.get(engine, uri_type) if custom_handler: diff --git a/src/xarray_prism/utils.py b/src/xarray_prism/utils.py index 8b4d964..7b3613d 100644 --- a/src/xarray_prism/utils.py +++ b/src/xarray_prism/utils.py @@ -161,8 +161,7 @@ def gdal_env(storage_options: Optional[Dict[str, Any]] = None) -> Iterator[None] [ "use_cftime", "decode_cf", - "decode_times", - "decode_timedelta", + "concat_characters", "use_default_fill_value", "cftime_variables", ] @@ -197,7 +196,7 @@ def sanitize_rasterio_kwargs(kwargs: Dict[str, Any]) -> Dict[str, Any]: for key, (allowed, reason) in _RASTERIO_RESTRICTED_KWARGS.items(): if key in sanitized and sanitized[key] != allowed: logger.warning( - "'%s=%r' is not supported — " "%s (using '%s' instead).", + "'%s=%r' is not supported — %s (using '%s' instead).", key, sanitized[key], reason, diff --git a/tests/conftest.py b/tests/conftest.py index a4d929c..0e91cae 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,11 +1,18 @@ from __future__ import annotations +import importlib.util +import inspect import os import tempfile +import warnings from pathlib import Path from typing import Generator +import numpy as np import pytest +import xarray as xr + +from xarray_prism._delegate import API_ONLY_KWARGS, DECODER_PARAMETERS PROJECT_ROOT = Path(__file__).parent.parent DATA_DIR = PROJECT_ROOT / "data" @@ -166,3 +173,360 @@ def is_service_available(host: str, port: int, timeout: float = 1.0) -> bool: item.add_marker(skip_thredds) if "requires_data" in item.keywords and not data_available: item.add_marker(skip_data) + + +# first-class open_dataset arguments fixtures, parametrization and probes + +XR_OPEN_DATASET_PARAMS = frozenset(inspect.signature(xr.open_dataset).parameters) + +_PASSTHROUGH_KWARGS = frozenset( + {"filename_or_obj", "drop_variables", *DECODER_PARAMETERS} +) + +_CONSUMED_KWARGS = frozenset({"decode_cf", "backend_kwargs", "kwargs"}) + + +def _module_available(name: str) -> bool: + return importlib.util.find_spec(name) is not None + + +def _needs_dask(reason: str) -> pytest.MarkDecorator: + return pytest.mark.skipif(not _module_available("dask"), reason=reason) + + +def _needs_open_dataset_arg(name: str) -> pytest.MarkDecorator: + return pytest.mark.skipif( + name not in XR_OPEN_DATASET_PARAMS, + reason=f"xarray {xr.__version__} has no open_dataset({name}=...)", + ) + + +_DECODER_CASES = { + "defaults": {}, + "decode_cf-False": {"decode_cf": False}, + "decode_cf-True": {"decode_cf": True}, + "mask_and_scale-False": {"mask_and_scale": False}, + "decode_times-False": {"decode_times": False}, + "decode_timedelta-True": {"decode_timedelta": True}, + "concat_characters-False": {"concat_characters": False}, + "decode_coords-all": {"decode_coords": "all"}, + "decode_coords-False": {"decode_coords": False}, + "drop_variables": {"drop_variables": "junk"}, + "cache-False": {"cache": False}, + "cache-True": {"cache": True}, + "create_default_indexes-False": {"create_default_indexes": False}, +} + +_RASTERIO_CASES = { + "defaults": {}, + "mask_and_scale-False": {"mask_and_scale": False}, + "decode_times-False": {"decode_times": False}, + "decode_coords-all": {"decode_coords": "all"}, +} + +_API_LAYER_CASES = { + "cache-False": {"cache": False}, + "cache-True": {"cache": True}, + "chunks": {"chunks": {}}, + "create_default_indexes": {"create_default_indexes": False}, + "inline_array": {"chunks": {}, "inline_array": True}, + "chunked_array_type": {"chunks": {}, "chunked_array_type": "dask"}, + "from_array_kwargs": {"chunks": {}, "from_array_kwargs": {}}, +} + +_CASE_MARKS = { + "create_default_indexes": (_needs_open_dataset_arg("create_default_indexes"),), + "create_default_indexes-False": ( + _needs_open_dataset_arg("create_default_indexes"), + ), + "chunks": (_needs_dask("chunks needs a chunk manager"),), + "inline_array": (_needs_dask("inline_array needs a chunk manager"),), + "chunked_array_type": (_needs_dask("chunked_array_type needs a chunk manager"),), + "from_array_kwargs": (_needs_dask("from_array_kwargs needs a chunk manager"),), +} + + +def _params(cases: dict) -> list: + return [ + pytest.param(kwargs, id=name, marks=list(_CASE_MARKS.get(name, ()))) + for name, kwargs in cases.items() + ] + + +def pytest_generate_tests(metafunc): + """Supply the argument matrices by fixture name.""" + if "decoder_kwargs" in metafunc.fixturenames: + metafunc.parametrize("decoder_kwargs", _params(_DECODER_CASES)) + if "rasterio_kwargs" in metafunc.fixturenames: + metafunc.parametrize("rasterio_kwargs", _params(_RASTERIO_CASES)) + if "api_layer_kwargs" in metafunc.fixturenames: + metafunc.parametrize("api_layer_kwargs", _params(_API_LAYER_CASES)) + if "api_only_kwarg" in metafunc.fixturenames: + metafunc.parametrize("api_only_kwarg", sorted(API_ONLY_KWARGS)) + if "cache_enabled" in metafunc.fixturenames: + metafunc.parametrize("cache_enabled", [True, False], ids=["cache", "nocache"]) + if "any_dataset" in metafunc.fixturenames: + metafunc.parametrize( + "any_dataset", + ["cf_file", "netcdf3_file", "zarr_store", "geotiff_file"], + indirect=True, + ) + + +@pytest.fixture(scope="session") +def cf_decoding_supported() -> None: + """ + Skip when the installed xarray/pandas pair cannot decode timedeltas. + """ + try: + var = xr.Variable(("t",), np.array([0, 6, 12]), {"units": "hours"}) + xr.conventions.decode_cf_variables({"t": var}, {})[0]["t"].values + except Exception as exc: # noqa: BLE001 + pytest.skip( + f"xarray {xr.__version__} cannot decode timedeltas with the " + f"installed pandas ({exc}); parity is untestable here" + ) + + +@pytest.fixture(scope="session") +def cf_file(tmp_path_factory, cf_decoding_supported) -> str: + """ + NetCDF4 file exercising every CF decoder, incl. conflicting fill values. + """ + nc = pytest.importorskip("netCDF4") + path = tmp_path_factory.mktemp("prism") / "cf.nc" + with nc.Dataset(path, "w") as ds: + ds.createDimension("time", 4) + ds.createDimension("x", 3) + ds.createDimension("strlen", 4) + + time = ds.createVariable("time", "f8", ("time",)) + time.units = "days since 2000-01-01" + time.calendar = "standard" + time[:] = [0, 1, 2, 3] + + lead = ds.createVariable("lead", "f8", ("time",)) + lead.units = "hours" + lead[:] = [0, 6, 12, 18] + + lon = ds.createVariable("lon", "f8", ("x",)) + lon[:] = [1.0, 2.0, 3.0] + + tas = ds.createVariable("tas", "i2", ("time", "x"), fill_value=np.int16(-32767)) + tas.scale_factor = 0.01 + tas.add_offset = 273.15 + tas.coordinates = "lon" + tas[:] = np.arange(12, dtype="i2").reshape(4, 3) + + temperature = ds.createVariable( + "temperature", "f4", ("time",), fill_value=np.float32(-9999.0) + ) + temperature.missing_value = np.float64(-8888.0) + temperature[:] = [280.0, 281.0, 282.0, 283.0] + + label = ds.createVariable("label", "S1", ("time", "strlen")) + label[:] = np.array( + [list(b"ab "), list(b"cd "), list(b"ef "), list(b"gh ")], dtype="S1" + ) + + junk = ds.createVariable("junk", "f8", ("x",)) + junk[:] = [9.0, 9.0, 9.0] + return str(path) + + +@pytest.fixture(scope="session") +def netcdf3_file(tmp_path_factory, cf_decoding_supported) -> str: + """NetCDF3 classic file -> scipy engine.""" + nc = pytest.importorskip("netCDF4") + pytest.importorskip("scipy") + path = tmp_path_factory.mktemp("prism") / "n3.nc" + with nc.Dataset(path, "w", format="NETCDF3_CLASSIC") as ds: + ds.createDimension("time", 3) + time = ds.createVariable("time", "f8", ("time",)) + time.units = "days since 2000-01-01" + time[:] = [0, 1, 2] + tas = ds.createVariable("tas", "i2", ("time",)) + tas.scale_factor = 0.5 + tas.add_offset = 100.0 + tas[:] = [1, 2, 3] + return str(path) + + +@pytest.fixture(scope="session") +def zarr_store(cf_file, tmp_path_factory) -> str: + """Zarr store -> zarr engine.""" + pytest.importorskip("zarr") + path = tmp_path_factory.mktemp("prism") / "z.zarr" + try: + with xr.open_dataset(cf_file, drop_variables="temperature") as ds: + ds.to_zarr(path, mode="w") + except Exception as exc: + pytest.skip(f"cannot write zarr with this xarray/zarr combination: {exc}") + return str(path) + + +@pytest.fixture(scope="session") +def geotiff_file(tmp_path_factory) -> str: + """Single-band uint8 GeoTIFF -> rasterio engine.""" + rasterio = pytest.importorskip("rasterio") + pytest.importorskip("rioxarray") + from rasterio.transform import from_origin + + path = tmp_path_factory.mktemp("prism") / "u8.tif" + with rasterio.open( + path, + "w", + driver="GTiff", + height=3, + width=3, + count=1, + dtype="uint8", + nodata=0, + crs="EPSG:4326", + transform=from_origin(0, 3, 1, 1), + ) as dst: + dst.write(np.arange(9, dtype="uint8").reshape(1, 3, 3)) + return str(path) + + +@pytest.fixture +def any_dataset(request) -> str: + """Indirect fixture: resolves a fixture *name* to its path.""" + return request.getfixturevalue(request.param) + + +@pytest.fixture(scope="session") +def fingerprint(): + """Comparable summary of an opened dataset.""" + + def _fingerprint(ds: xr.Dataset) -> dict: + return { + "dtypes": {n: str(v.dtype) for n, v in ds.variables.items()}, + "coords": sorted(ds.coords), + "indexes": sorted(ds.xindexes), + "values": { + n: repr(np.asarray(v.values).ravel()[:3]) + for n, v in sorted(ds.variables.items()) + }, + } + + return _fingerprint + + +@pytest.fixture(scope="session") +def wrapper_chain(): + """ + Lazy-array wrappers around a variable, outermost first. + """ + + def _wrapper_chain(ds: xr.Dataset, name: str) -> list: + node = ds[name].variable._data + chain = [] + for _ in range(10): + chain.append(type(node).__name__) + node = getattr(node, "array", None) + if node is None: + break + return chain + + return _wrapper_chain + + +@pytest.fixture(scope="session") +def open_probe(fingerprint): + """ + Open a dataset and report (fingerprint or exception, warning categories). + """ + + def _open_probe(path: str, engine: str, kwargs: dict) -> tuple: + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + try: + with xr.open_dataset(path, engine=engine, **kwargs) as ds: + result = fingerprint(ds) + except Exception as exc: + result = f"raised {type(exc).__name__}" + return result, sorted({w.category.__name__ for w in caught}) + + return _open_probe + + +@pytest.fixture(scope="session") +def assert_parity(open_probe): + """ + Assert prism matches the engine it delegates to, warnings included. + """ + + def _assert_parity(path: str, delegate: str, kwargs: dict) -> None: + expected = open_probe(path, delegate, kwargs) + actual = open_probe(path, "prism", kwargs) + assert actual == expected, ( + f"\n kwargs : {kwargs}" + f"\n {delegate:<9}: {expected}" + f"\n prism : {actual}" + ) + + return _assert_parity + + +class _RecordingBackend: + """ + Stands in for the delegate; records what prism hands it, then defers. + """ + + def __init__(self, real): + self._real = real + self.captured: dict | None = None + + def __getattr__(self, name): + return getattr(self._real, name) + + def open_dataset(self, filename_or_obj, **kwargs): + self.captured = dict(kwargs) + return self._real.open_dataset(filename_or_obj, **kwargs) + + +@pytest.fixture +def recorder(monkeypatch) -> _RecordingBackend: + """ + Intercept prism's delegate lookup, not xarray's lookup of prism. + """ + pytest.importorskip("h5netcdf") + from xarray.backends import plugins + + real_get_backend = plugins.get_backend + rec = _RecordingBackend(real_get_backend("h5netcdf")) + + def scoped_get_backend(engine): + if engine == "prism" or not isinstance(engine, str): + return real_get_backend(engine) + return rec + + monkeypatch.setattr(plugins, "get_backend", scoped_get_backend) + return rec + + +@pytest.fixture(scope="session") +def open_dataset_arguments() -> frozenset: + """Every first-class argument of this xarray's open_dataset.""" + return XR_OPEN_DATASET_PARAMS + + +@pytest.fixture(scope="session") +def classified_kwargs() -> dict: + """How xarray-prism accounts for + each open_dataset argument.""" + return { + "api_only": API_ONLY_KWARGS, + "passthrough": _PASSTHROUGH_KWARGS, + "consumed": _CONSUMED_KWARGS, + } + + +@pytest.fixture +def quiet(): + """Suppress warnings that are irrelevant + to the assertion under test.""" + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + yield diff --git a/tests/test_first_class_args.py b/tests/test_first_class_args.py new file mode 100644 index 0000000..0e8e690 --- /dev/null +++ b/tests/test_first_class_args.py @@ -0,0 +1,227 @@ +""" +test xarray's first-class ``open_dataset`` arguments +""" + +from __future__ import annotations + +import logging + +import numpy as np +import pytest +import xarray as xr + +from xarray_prism import PrismBackendEntrypoint +from xarray_prism._delegate import API_ONLY_KWARGS, DECODER_PARAMETERS + + +class TestParity: + """prism must produce what the delegate + engine produces, warnings included.""" + + def test_netcdf4(self, cf_file, decoder_kwargs, assert_parity): + pytest.importorskip("h5netcdf") + assert_parity(cf_file, "h5netcdf", decoder_kwargs) + + def test_netcdf3(self, netcdf3_file, decoder_kwargs, assert_parity): + assert_parity(netcdf3_file, "scipy", decoder_kwargs) + + def test_zarr(self, zarr_store, decoder_kwargs, assert_parity): + assert_parity(zarr_store, "zarr", decoder_kwargs) + + def test_geotiff(self, geotiff_file, rasterio_kwargs, assert_parity): + assert_parity(geotiff_file, "rasterio", rasterio_kwargs) + + def test_open_dataarray(self, cf_file, quiet): + pytest.importorskip("h5netcdf") + drop = ["tas", "junk", "lead", "label", "lon"] + for kwargs in ({}, {"decode_cf": False}): + with xr.open_dataarray( + cf_file, engine="h5netcdf", drop_variables=drop, **kwargs + ) as expected: + with xr.open_dataarray( + cf_file, engine="prism", drop_variables=drop, **kwargs + ) as actual: + assert actual.dtype == expected.dtype + + def test_open_mfdataset(self, cf_file, quiet): + pytest.importorskip("dask") + pytest.importorskip("h5netcdf") + for kwargs in ({}, {"decode_cf": False}): + with xr.open_mfdataset([cf_file], engine="h5netcdf", **kwargs) as expected: + with xr.open_mfdataset([cf_file], engine="prism", **kwargs) as actual: + assert actual["tas"].dtype == expected["tas"].dtype + + +class TestOriginalReport: + """https://github.com/freva-org/xarray-prism -- decode_cf=False ignored.""" + + def test_no_serialization_warning(self, cf_file, recwarn): + from xarray.coding.variables import SerializationWarning + + with xr.open_dataset(cf_file, engine="prism", decode_cf=False) as ds: + assert ds["tas"].dtype == np.int16 + assert ds["time"].dtype == np.float64 + + offenders = [w for w in recwarn if issubclass(w.category, SerializationWarning)] + assert not offenders, f"unexpected SerializationWarning(s): {offenders}" + + def test_decoders_are_advertised(self): + """xarray only expands decode_cf for the names a backend advertises.""" + declared = set(PrismBackendEntrypoint.open_dataset_parameters) + assert set(DECODER_PARAMETERS) <= declared + assert {"filename_or_obj", "drop_variables"} <= declared + + +class TestCacheWrapping: + """cache is applied by the API layer + and must not be overridden early.""" + + def test_wrapper_chain_matches_delegate( + self, cf_file, cache_enabled, wrapper_chain, quiet + ): + pytest.importorskip("h5netcdf") + with xr.open_dataset( + cf_file, engine="h5netcdf", cache=cache_enabled + ) as expected: + reference = wrapper_chain(expected, "tas") + with xr.open_dataset(cf_file, engine="prism", cache=cache_enabled) as actual: + observed = wrapper_chain(actual, "tas") + + assert observed == reference + assert ("MemoryCachedArray" in observed) is cache_enabled + + +class TestDelegationBoundary: + """What prism hands the real backend, + asserted at the handover itself.""" + + def test_api_layer_args_never_reach_delegate( + self, cf_file, recorder, api_layer_kwargs, quiet + ): + xr.open_dataset(cf_file, engine="prism", **api_layer_kwargs).close() + + assert recorder.captured is not None, "delegate was never called" + leaked = API_ONLY_KWARGS & recorder.captured.keys() + assert not leaked, f"API-layer args leaked into the delegate: {sorted(leaked)}" + assert "decode_cf" not in recorder.captured, "decode_cf is not a backend param" + assert "backend_kwargs" not in recorder.captured + + def test_decoders_do_reach_delegate(self, cf_file, recorder, quiet): + """The flip side: decode_cf must + arrive resolved, not dropped.""" + xr.open_dataset(cf_file, engine="prism", decode_cf=False).close() + + assert recorder.captured is not None + unresolved = { + name: recorder.captured.get(name) + for name in DECODER_PARAMETERS + if recorder.captured.get(name) is not False + } + assert not unresolved, f"decode_cf=False did not resolve these: {unresolved}" + + def test_direct_helpers_return_raw_backend_dataset( + self, cf_file, fingerprint, wrapper_chain, quiet + ): + """ + open_posix / open_cloud / PrismBackendEntrypoint.open_dataset + return an undecorated backend dataset, like every other + BackendEntrypoint + """ + pytest.importorskip("h5netcdf") + from xarray.backends.plugins import get_backend + + from xarray_prism.backends import open_posix + + actual = open_posix(cf_file, engine="h5netcdf") + reference = get_backend("h5netcdf").open_dataset(cf_file) + try: + assert list(actual.xindexes) == list(reference.xindexes) + assert fingerprint(actual) == fingerprint(reference) + assert "MemoryCachedArray" not in wrapper_chain(actual, "tas") + finally: + actual.close() + reference.close() + + with xr.open_dataset(cf_file, engine="prism") as full: + assert list(full.xindexes) == ["time"] + + +class TestBackendKwargs: + """backend_kwargs is flattened by xarray; + prism must cope predictably.""" + + def test_decode_cf_matches_first_class_form(self, any_dataset, open_probe): + """ + the rasterio decode_cf + """ + first_class = open_probe(any_dataset, "prism", {"decode_cf": False}) + nested = open_probe( + any_dataset, "prism", {"backend_kwargs": {"decode_cf": False}} + ) + assert nested == first_class + + def test_decode_cf_is_allowed(self, cf_file, quiet): + """The one deliberate exception + must survive the API-only check.""" + pytest.importorskip("h5netcdf") + with xr.open_dataset( + cf_file, engine="prism", backend_kwargs={"decode_cf": False} + ) as ds: + assert ds["tas"].dtype == np.int16 + + def test_api_only_args_raise_clearly(self, cf_file, api_only_kwarg): + with pytest.raises(TypeError, match="must be passed directly"): + xr.open_dataset( + cf_file, engine="prism", backend_kwargs={api_only_kwarg: None} + ) + + +class TestRasterioDivergence: + """prism deliberately adapts rather + than propagating rioxarray's error.""" + + def test_decode_cf_succeeds_where_rioxarray_raises(self, geotiff_file, caplog): + from rioxarray.exceptions import RioXarrayError + + with pytest.raises(RioXarrayError, match="decode_coords"): + xr.open_dataset(geotiff_file, engine="rasterio", decode_cf=False) + + with caplog.at_level(logging.WARNING, logger="xarray_prism"): + with xr.open_dataset(geotiff_file, engine="prism", decode_cf=False) as ds: + assert ds["band_data"].dtype == np.uint8 + assert "decode_coords" in caplog.text + + def test_dropped_kwargs_are_logged(self, geotiff_file, caplog): + """Discarding a user argument must at least be reported.""" + with caplog.at_level(logging.WARNING, logger="xarray_prism"): + xr.open_dataset( + geotiff_file, engine="prism", concat_characters=False + ).close() + assert "concat_characters" in caplog.text + + +class TestSignatureDrift: + """A new xarray argument must be classified, not silently ignored.""" + + def test_no_unclassified_open_dataset_arguments( + self, open_dataset_arguments, classified_kwargs + ): + """Each argument is either handled by xarray's API layer, forwarded to + the delegate, or consumed en route. + """ + known = frozenset().union(*classified_kwargs.values()) + unclassified = open_dataset_arguments - known + assert not unclassified, ( + f"xarray {xr.__version__} has open_dataset argument(s) " + f"{sorted(unclassified)} that xarray-prism has not classified. " + f"Decide whether each is API-layer-only, forwarded to the delegate, " + f"or consumed by prism, then update the corresponding set." + ) + + def test_advertised_parameters_are_real_backend_parameters(self): + advertised = set(PrismBackendEntrypoint.open_dataset_parameters) + assert not advertised & API_ONLY_KWARGS, ( + "open_dataset_parameters advertises an API-layer-only argument: " + f"{sorted(advertised & API_ONLY_KWARGS)}" + ) + assert "decode_cf" not in advertised, "decode_cf is never a backend parameter"