Merge pull request #48 from noaaroland/quote

Some fixes for OPeNDAP attributes

noxfile.py

@@ -13,8 +13,12 @@
 @nox.session(python=python_versions)
 @nox.parametrize("pydantic", pydantic_versions)
 def tests(session: nox.Session, pydantic: str):
-    """Run py.test against Github Actions matrix."""
+    """Run py.test against Github Actions matrix.
+
+    Allows passing additional arguments to py.test with with postargs,
+    for example: `nox -- --pdb`.
+    """
     session.install("-r", "requirements-dev.txt")
     session.install(".")
     session.install(f"pydantic{pydantic}")
-    session.run("pytest", "--verbose")
+    session.run("pytest", "--verbose", *session.posargs)

pyproject.toml

@@ -4,10 +4,10 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "xpublish_opendap"
-description = ""
+description = "OpenDAP plugin for Xpublish"
 readme = "README.md"
 requires-python = ">=3.9"
-keywords = []
+keywords = ["xarray", "xpublish", "opendap"]
 license = { file = "LICENSE.txt" }
 
 classifiers = [
@@ -43,7 +43,7 @@ write_to = "xpublish_opendap/_version.py"
 [tool.check-manifest]
 ignore = ["xpublish_opendap/_version.py"]
 
-[tool.ruff]
+[tool.ruff.lint]
 select = [
     "A",   # flake8-builtins
     "B",   # flake8-bugbear
@@ -58,10 +58,15 @@ select = [
     "W",   # pycodestyle warnings
 ]
 
-[tool.ruff.pydocstyle]
+[tool.ruff.lint.extend-per-file-ignores]
+"tests/*" = [
+    "PLR2004"  # It's reasonable to use magic values in tests
+    ]
+
+[tool.ruff.lint.pydocstyle]
 # Use Google-style docstrings.
 convention = "google"
 
-[tool.ruff.flake8-bugbear]
+[tool.ruff.lint.flake8-bugbear]
 # Allow fastapi.Depends and other dependency injection style function arguments
 extend-immutable-calls = ["fastapi.Depends", "fastapi.Query"]

requirements-dev.txt

@@ -1,18 +1,22 @@
 black
 check-manifest
 doctr
+h5netcdf
+h5pyd
 httpx
 nbsphinx
 netCDF4
 nox
 pooch
 pre-commit
+pydap
 pylint
 pytest
 pytest-cov
 pytest-flake8
 pytest-github-actions-annotate-failures
 pytest-xdist
+pytest-xprocess
 recommonmark
 ruff
 setuptools_scm

tests/conftest.py

@@ -0,0 +1,46 @@
+"""Py.test configuration and shared fixtures."""
+
+from pathlib import Path
+
+import pytest
+from xprocess import ProcessStarter
+
+server_path = Path(__file__).parent / "server.py"
+
+
+@pytest.fixture
+def xpublish_server(xprocess):
+    """Launch an Xpublish server in the background.
+
+    Server has the air_temperature tutorial dataset
+    at `air` and has the OpenDAP plugin running with
+    defaults.
+    """
+
+    class Starter(ProcessStarter):
+        # Wait till the pattern is printed before
+        # considering things started
+        pattern = "Uvicorn running on"
+
+        # server startup args
+        args = ["python", str(server_path)]
+
+        # seconds before timing out on server startup
+        timeout = 30
+
+        # Try to cleanup if inturrupted
+        terminate_on_interrupt = True
+
+    xprocess.ensure("xpublish", Starter)
+    yield "http://0.0.0.0:9000"
+    xprocess.getinfo("xpublish").terminate()
+
+
+@pytest.fixture(scope="session")
+def dataset():
+    """Xarray air temperature tutorial dataset."""
+    from xarray.tutorial import open_dataset
+
+    ds = open_dataset("air_temperature")
+
+    return ds

tests/server.py

@@ -0,0 +1,22 @@
+"""Test OpenDAP server with air temperature dataset."""
+
+import numpy as np
+import xarray.tutorial
+import xpublish
+
+from xpublish_opendap import OpenDapPlugin
+
+ds = xarray.tutorial.open_dataset("air_temperature")
+
+ds_attrs_quote = xarray.tutorial.open_dataset("air_temperature")
+ds_attrs_quote.attrs["quotes"] = 'This attribute uses "quotes"'
+ds_attrs_cast = xarray.tutorial.open_dataset("air_temperature")
+ds_attrs_cast.attrs["npint"] = np.int16(16)
+ds_attrs_cast.attrs["npintthirtytwo"] = np.int32(32)
+
+rest = xpublish.Rest(
+    {"air": ds, "attrs_quote": ds_attrs_quote, "attrs_cast": ds_attrs_cast},
+    plugins={"opendap": OpenDapPlugin()},
+)
+
+rest.serve()

tests/test_opendap_router.py

@@ -6,15 +6,6 @@
 from xpublish_opendap import OpenDapPlugin
 
 
-@pytest.fixture(scope="session")
-def dataset():
-    from xarray.tutorial import open_dataset
-
-    ds = open_dataset("air_temperature")
-
-    return ds
-
-
 @pytest.fixture(scope="session")
 def dap_xpublish(dataset):
     rest = xpublish.Rest({"air": dataset}, plugins={"opendap": OpenDapPlugin()})
@@ -42,7 +33,7 @@ def test_dds_response(dap_client):
     assert "Float32 time[time = 2920]" in content
     assert "Float32 lon[lon = 53]" in content
     assert "Grid {" in content
-    assert "Float32 air[time = 2920][lat = 25][lon = 53]" in content
+    assert "Float64 air[time = 2920][lat = 25][lon = 53]" in content
 
 
 def test_das_response(dap_client):
@@ -77,4 +68,4 @@ def test_dods_response(dap_client):
     assert "Float32 time[time = 2920]" in text_content
     assert "Float32 lon[lon = 53]" in text_content
     assert "Grid {" in text_content
-    assert "Float32 air[time = 2920][lat = 25][lon = 53]" in text_content
+    assert "Float64 air[time = 2920][lat = 25][lon = 53]" in text_content

tests/test_server.py

@@ -0,0 +1,78 @@
+"""Test OpenDAP clients against Xpublish OpenDAP plugin.
+
+Live tests are currently failing on Windows, see:
+- https://github.com/Unidata/netcdf-c/issues/2459
+- https://github.com/Unidata/netcdf4-python/issues/1246
+- https://github.com/pydata/xarray/issues/7773
+"""
+
+import sys
+
+import netCDF4
+import pytest
+import xarray as xr
+
+
+@pytest.mark.skipif(
+    sys.platform == "win32",
+    reason="NetCDF4 is failing on Windows Github Actions workers",
+)
+def test_netcdf4(xpublish_server):
+    """Test opening OpenDAP air dataset directly with NetCDF4 library."""
+    url = f"{xpublish_server}/datasets/air/opendap"
+    netCDF4.Dataset(url)
+
+
+@pytest.mark.skipif(
+    sys.platform == "win32",
+    reason="NetCDF4 is failing on Windows Github Actions workers",
+)
+def test_default_xarray_engine(xpublish_server, dataset):
+    """Test opening OpenDAP air dataset with default Xarray engine."""
+    url = f"{xpublish_server}/datasets/air/opendap"
+    ds = xr.open_dataset(url)
+    assert ds == dataset
+
+
+@pytest.mark.skipif(
+    sys.platform == "win32",
+    reason="NetCDF4 is failing on Windows Github Actions workers",
+)
+@pytest.mark.parametrize(
+    "engine",
+    [
+        "netcdf4",
+        # "h5netcdf", # fails with 404 not found
+        # "pydap"  # fails with incomplete read
+    ],
+)
+def test_xarray_engines(xpublish_server, engine, dataset):
+    """Test opening OpenDAP dataset with specified engines."""
+    url = f"{xpublish_server}/datasets/air/opendap"
+    ds = xr.open_dataset(url, engine=engine)
+    assert ds == dataset
+
+
+@pytest.mark.skipif(
+    sys.platform == "win32",
+    reason="NetCDF4 is failing on Windows Github Actions workers",
+)
+def test_attrs_quotes(xpublish_server):
+    """Test that we are formatting OpenDAP attributes that contain '"' properly."""
+    url = f"{xpublish_server}/datasets/attrs_quote/opendap"
+    ds = xr.open_dataset(url)
+
+    assert ds.attrs["quotes"] == 'This attribute uses "quotes"'
+
+
+@pytest.mark.skipif(
+    sys.platform == "win32",
+    reason="NetCDF4 is failing on Windows Github Actions workers",
+)
+def test_attrs_types(xpublish_server):
+    """Test that we are formatting OpenDAP attributes that contain '"' properly."""
+    url = f"{xpublish_server}/datasets/attrs_cast/opendap"
+    ds = xr.open_dataset(url)
+
+    assert ds.attrs["npint"] == 16
+    assert ds.attrs["npintthirtytwo"] == 32

xpublish_opendap/dap_xarray.py

@@ -47,6 +47,17 @@ def dap_attribute(key: str, value: Any) -> dap.Attribute:
         dtype = dap.Int32
     elif isinstance(value, float):
         dtype = dap.Float64
+    elif isinstance(value, np.float32):
+        dtype = dap.Float32
+    elif isinstance(value, np.int16):
+        dtype = dap.Int16
+    elif isinstance(value, np.int32):
+        dtype = dap.Int32
+    elif isinstance(value, str):
+        dtype = dap.String
+        # Escape a double quote in the attribute value.
+        # Other servers like TDS do this. Without this clients fail.
+        value = value.replace('"', '\\"')
     else:
         dtype = dap.String