From dab78a217fad1723c63ec0c88e84c43398ebcfb2 Mon Sep 17 00:00:00 2001
From: thodson-usgs <thodson@usgs.gov>
Date: Mon, 1 Jun 2026 14:45:15 -0400
Subject: [PATCH] fix(wqp): WQP_Metadata.variable_info returns None instead of
 raising

WQP_Metadata defines site_info but not variable_info, so accessing
`wqp_md.variable_info` fell through to the BaseMetadata stub and raised
NotImplementedError -- even though WQP simply has no variable/parameter
catalog. Override it to return None (matching how site_info returns None
when absent).

Also document on BaseMetadata that site_info/variable_info are legacy hooks
overridden by the nwis/wqp metadata subclasses and are not part of the
modern waterdata contract -- BaseMetadata is a concrete value object the
waterdata getters return directly, so it stays concrete (not an ABC).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 dataretrieval/utils.py | 11 +++++++++--
 dataretrieval/wqp.py   |  7 +++++++
 tests/wqp_test.py      |  7 +++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/dataretrieval/utils.py b/dataretrieval/utils.py
index 7bb03a69..3e6f4e84 100644
--- a/dataretrieval/utils.py
+++ b/dataretrieval/utils.py
@@ -202,7 +202,15 @@ def _attach_datetime_columns(df: pd.DataFrame) -> pd.DataFrame:
 
 
 class BaseMetadata:
-    """Base class for metadata.
+    """Base class for the metadata returned alongside a service's data.
+
+    A concrete value object holding the response URL, query time, and headers;
+    the modern ``waterdata`` getters return it directly.
+
+    ``site_info`` and ``variable_info`` are legacy hooks: the ``nwis`` / ``wqp``
+    metadata subclasses override them to look up site (or, historically,
+    variable) details for the query. They are not part of the modern
+    ``waterdata`` contract, so on the base they raise ``NotImplementedError``.
 
     Attributes
     ----------
@@ -212,7 +220,6 @@ class BaseMetadata:
         Response elapsed time
     header: httpx.Headers
         Response headers
-
     """
 
     def __init__(self, response) -> None:
diff --git a/dataretrieval/wqp.py b/dataretrieval/wqp.py
index 0e0adc8d..3826986d 100644
--- a/dataretrieval/wqp.py
+++ b/dataretrieval/wqp.py
@@ -676,6 +676,13 @@ def site_info(self) -> tuple[DataFrame, WQP_Metadata] | None:
             return None
         return what_sites(siteid=siteid)
 
+    @property
+    def variable_info(self) -> None:
+        """WQP exposes no variable/parameter catalog through metadata, so this
+        is always ``None`` (overriding the :class:`~dataretrieval.utils.BaseMetadata`
+        stub, which would otherwise raise ``NotImplementedError``)."""
+        return None
+
 
 def _check_kwargs(kwargs):
     """Private function to check kwargs for unsupported parameters."""
diff --git a/tests/wqp_test.py b/tests/wqp_test.py
index 83b69ff4..f86b7f5e 100644
--- a/tests/wqp_test.py
+++ b/tests/wqp_test.py
@@ -272,6 +272,13 @@ def test_wqp_metadata_site_info_is_accessible_property():
     assert _wqp_metadata().site_info is None  # must NOT raise
 
 
+def test_wqp_metadata_variable_info_is_none():
+    """Regression: WQP has no variable catalog, so ``variable_info`` must
+    return ``None`` rather than inheriting (and raising) the
+    ``BaseMetadata.variable_info`` NotImplementedError stub."""
+    assert _wqp_metadata().variable_info is None  # must NOT raise
+
+
 def test_wqp_metadata_site_info_routes_to_what_sites(monkeypatch):
     """When the query carried a ``siteid`` (WQP's site identifier),
     ``site_info`` delegates to ``wqp.what_sites`` with that identifier."""