fix(waterdata): repair queryables fallback, scalar properties, sediment unit

Review fixes for the hash-drop path:

* get_ogc_data's queryables-fallback except caught requests.HTTPError /
  requests.RequestException, but the module imports only httpx (no requests
  import), so any queryables failure raised NameError instead of falling
  back. Catch (httpx.HTTPError, RuntimeError, ValueError) -- the types the
  path actually raises (RateLimited/ServiceUnavailable subclass RuntimeError).
* Guard _service_queryables against a non-dict 200 body so a malformed
  response falls back (caught ValueError) rather than escaping as
  AttributeError.
* Normalize a scalar-string `properties` to a list at the get_ogc_data
  boundary, so get_reference_table's raw query dict can't trip
  `all(pd.isna("..."))` (TypeError) or a per-character set in _arrange_cols.
* CF_UNIT_MAP: "tons/day" -> "ton day-1" ("short_ton" is not a valid UDUNITS
  name; UDUNITS "ton" is the US short ton).

Add regression tests for the queryables fallback and scalar-properties paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: thodson-usgs

dataretrieval/waterdata/types.py

@@ -95,7 +95,8 @@
     "uS/cm": "uS/cm",
     "mg/l": "mg L-1",
     "mg/L": "mg L-1",
-    "tons/day": "short_ton day-1",
+    # UDUNITS 'ton' is the US short ton; 'short_ton' is not a valid UDUNITS name.
+    "tons/day": "ton day-1",
     "%": "percent",
 }
 

dataretrieval/waterdata/utils.py

@@ -245,15 +245,19 @@ def _service_queryables(service: str) -> list[str]:
     """Return the cached queryables property list for ``service``.
 
     One HTTP GET per service per process; the list is reused for every
-    subsequent call. Raises ``requests.HTTPError`` on a non-200 — the
-    caller's ``include_hash=False`` request can't be satisfied
-    without it, so failing loudly is preferable to silently dropping
-    the server-side trim.
+    subsequent call. Raises on a non-200 (a ``RuntimeError`` subclass via
+    ``_raise_for_non_200``, or an ``httpx.HTTPError`` on a transport failure);
+    ``get_ogc_data`` catches those and falls back to the client-side drop.
     """
     cached = _queryables_cache.get(service)
     if cached is not None:
         return cached
     body = _check_ogc_requests(endpoint=service, req_type="queryables")
+    if not isinstance(body, dict):
+        # A 200 with a non-object body would make ``body.get`` raise
+        # AttributeError, which get_ogc_data's fallback doesn't catch; raise a
+        # caught type so a malformed response falls back to the client-side drop.
+        raise ValueError(f"queryables response for {service!r} was not a JSON object")
     props = list(body.get("properties", {}).keys())
     _queryables_cache[service] = props
     return props
@@ -1427,8 +1431,14 @@ def get_ogc_data(
     args["service"] = service
     args = _switch_arg_id(args, id_name=output_id, service=service)
     # Capture `properties` before the id-switch so post-processing sees
-    # the user-facing names, not the wire-format ones.
+    # the user-facing names, not the wire-format ones. A scalar string
+    # (e.g. from ``get_reference_table``'s raw ``query`` dict, which skips
+    # the list-normalization the typed getters do) is wrapped to a list so
+    # the downstream predicates / set-builds treat it as one column rather
+    # than per-character.
     properties = args.get("properties")
+    if isinstance(properties, str):
+        properties = [properties]
     convert_type = args.pop("convert_type", False)
     include_hash = args.pop("include_hash", False)
 
@@ -1441,7 +1451,7 @@ def get_ogc_data(
     if use_server_trim:
         try:
             args["properties"] = _default_non_hash_properties(service, output_id)
-        except (requests.HTTPError, requests.RequestException, ValueError) as exc:
+        except (httpx.HTTPError, RuntimeError, ValueError) as exc:
             logger.warning(
                 "Could not fetch queryables for %s (%s); "
                 "falling back to client-side hash-ID drop.",

tests/waterdata_utils_test.py

@@ -927,3 +927,49 @@ def test_check_ogc_requests_raises_typed_on_5xx(httpx_mock):
     )
     with pytest.raises(ServiceUnavailable):
         _check_ogc_requests(endpoint="daily", req_type="schema")
+
+
+def _reach_fetch_sentinel(monkeypatch):
+    """Stub ``_fetch_once`` to raise a recognizable sentinel, so a test can
+    prove ``get_ogc_data`` got past the properties/queryables handling (rather
+    than crashing earlier) without doing any network I/O."""
+
+    def _boom(_args):
+        raise RuntimeError("reached _fetch_once")
+
+    monkeypatch.setattr(_utils_module, "_fetch_once", _boom)
+
+
+def test_get_ogc_data_falls_back_when_queryables_unavailable(monkeypatch):
+    # A queryables fetch failure (ServiceUnavailable / httpx error) must be
+    # caught and fall through to the client-side drop -- not crash. Regression:
+    # the except clause referenced an unimported ``requests``, so any failure
+    # raised NameError instead of falling back.
+    def _boom_queryables(*args, **kwargs):
+        raise ServiceUnavailable("503")
+
+    monkeypatch.setattr(_utils_module, "_default_non_hash_properties", _boom_queryables)
+    _reach_fetch_sentinel(monkeypatch)
+    # Reaching the sentinel proves the queryables error was caught and the call
+    # continued; a NameError or the bare ServiceUnavailable("503") would not
+    # match and would fail the test.
+    with pytest.raises(RuntimeError, match="reached _fetch_once"):
+        _utils_module.get_ogc_data(
+            args={"monitoring_location_id": "USGS-1"},
+            output_id="daily_id",
+            service="daily",
+        )
+
+
+def test_get_ogc_data_accepts_scalar_properties(monkeypatch):
+    # A scalar (non-list) ``properties`` -- reachable via get_reference_table's
+    # raw query dict -- must be treated as a single column, not crash
+    # ``_properties_unspecified`` (all(pd.isna("time")) -> TypeError) or become
+    # a per-character set in ``_arrange_cols``.
+    _reach_fetch_sentinel(monkeypatch)
+    with pytest.raises(RuntimeError, match="reached _fetch_once"):
+        _utils_module.get_ogc_data(
+            args={"properties": "time", "monitoring_location_id": "USGS-1"},
+            output_id="daily_id",
+            service="daily",
+        )