refactor(waterdata): emit the geopandas advisory once at import, not per call

Geopandas availability is a static, environment-level fact, so warn once at
module import (right where GEOPANDAS is determined) instead of inside the
getters/_paginate. This removes the per-call _warn_geopandas_once() latch and
guarantees the advisory prints exactly once — above any progress line — without
repeating per query/chunk or interleaving with the status line.

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

Author: thodson-usgs

dataretrieval/waterdata/utils.py

@@ -40,29 +40,22 @@
 # Set up logger for this module
 logger = logging.getLogger(__name__)
 
+# Whether geopandas is present is a static, environment-level fact, so warn once
+# here at import time rather than per query/chunk. That avoids the warning
+# repeating on every call and avoids it interleaving with the progress line's
+# carriage-return rewrites.
+if not GEOPANDAS:
+    logger.warning(
+        "Geopandas not installed. Geometries will be flattened into pandas DataFrames."
+    )
+
 BASE_URL = "https://api.waterdata.usgs.gov"
 OGC_API_VERSION = "v0"
 OGC_API_URL = f"{BASE_URL}/ogcapi/{OGC_API_VERSION}"
 SAMPLES_URL = f"{BASE_URL}/samples-data"
 STATISTICS_API_VERSION = "v0"
 STATISTICS_API_URL = f"{BASE_URL}/statistics/{STATISTICS_API_VERSION}"
 
-# "Geopandas not installed" is a static, environment-level fact. Emit it at most
-# once per process (and before any progress line opens) so it neither repeats
-# per query/chunk nor interleaves with the status line's carriage-return rewrites.
-_geopandas_warned = False
-
-
-def _warn_geopandas_once() -> None:
-    """Warn (once per process) that geometries are flattened without geopandas."""
-    global _geopandas_warned
-    if not GEOPANDAS and not _geopandas_warned:
-        _geopandas_warned = True
-        logger.warning(
-            "Geopandas not installed. Geometries will be flattened "
-            "into pandas DataFrames."
-        )
-
 
 def _switch_arg_id(ls: dict[str, Any], id_name: str, service: str):
     """
@@ -1257,7 +1250,6 @@ def get_ogc_data(
     convert_type = args.pop("convert_type", False)
     args = {k: v for k, v in args.items() if v is not None}
 
-    _warn_geopandas_once()  # before the progress line, so it never interleaves
     with _progress.progress_context(service=service):
         return_list, response = _fetch_once(args)
     return_list = _deal_with_empty(return_list, properties, service)
@@ -1332,9 +1324,8 @@ def _handle_stats_nesting(
     if not features:
         return gpd.GeoDataFrame() if geopd else pd.DataFrame()
 
-    # The geopd-missing warning is emitted once per process by
-    # ``_warn_geopandas_once`` at the getter entry; doing it here would log
-    # per page.
+    # The geopd-missing warning is emitted once at import (see top of module);
+    # doing it here would log per page.
     if not geopd:
         outer_props = [
             {k: v for k, v in (f.get("properties") or {}).items() if k != "data"}
@@ -1505,7 +1496,6 @@ def follow_up(cursor: str, sess: requests.Session) -> requests.Response:
 
     # The stats path doesn't go through ``multi_value_chunked``, so it opens
     # its own progress context; ``_paginate`` reports pages/rate-limit into it.
-    _warn_geopandas_once()  # before the progress line, so it never interleaves
     with _progress.progress_context(service=service):
         df, response = _paginate(
             req,

tests/waterdata_utils_test.py

@@ -406,66 +406,22 @@ def test_handle_stats_nesting_tolerates_missing_drop_columns():
     assert df["monitoring_location_id"].iloc[0] == "USGS-12345"
 
 
-def _warning_messages(caplog):
-    """Pull WARNING-level message strings out of caplog."""
-    return [r.getMessage() for r in caplog.records if r.levelno == logging.WARNING]
-
-
-@pytest.fixture(autouse=True)
-def _reset_geopandas_warned(monkeypatch):
-    """The geopandas advisory is latched once per process; reset it so these
-    warning tests don't depend on order."""
-    monkeypatch.setattr(_utils_module, "_geopandas_warned", False)
-
-
-def test_warn_geopandas_once_is_latched(monkeypatch, caplog):
-    # Static environment fact -> warn at most once per process.
-    monkeypatch.setattr(_utils_module, "GEOPANDAS", False)
-    caplog.set_level(logging.WARNING, logger=_LOGGER_NAME)
-    _utils_module._warn_geopandas_once()
-    _utils_module._warn_geopandas_once()  # second call must not re-warn
-    geo = [m for m in _warning_messages(caplog) if "Geopandas not installed" in m]
-    assert len(geo) == 1
-
-
-def test_warn_geopandas_once_silent_when_installed(monkeypatch, caplog):
-    monkeypatch.setattr(_utils_module, "GEOPANDAS", True)
-    caplog.set_level(logging.WARNING, logger=_LOGGER_NAME)
-    _utils_module._warn_geopandas_once()
-    assert not [m for m in _warning_messages(caplog) if "Geopandas not installed" in m]
-
-
-def test_get_stats_data_warns_once_when_geopandas_missing(caplog, monkeypatch):
-    # The advisory fires once per process (via _warn_geopandas_once) before the
-    # progress line — not once per paginated page, and not interleaved with it.
-    from dataretrieval.waterdata.utils import get_stats_data
-
-    monkeypatch.setattr(_utils_module, "GEOPANDAS", False)
-    monkeypatch.setattr(
-        _utils_module,
-        "_handle_stats_nesting",
-        mock.MagicMock(return_value=pd.DataFrame()),
+def test_geopandas_advisory_emitted_once_on_import():
+    # The advisory is a one-time module-import side effect, not per call. Import
+    # the module in a subprocess with geopandas blocked and confirm exactly one
+    # warning reaches stderr.
+    import subprocess
+    import sys
+
+    code = (
+        "import sys; sys.modules['geopandas'] = None\n"
+        "import dataretrieval.waterdata.utils\n"
     )
-    caplog.set_level(logging.WARNING, logger=_LOGGER_NAME)
-
-    page1 = mock.MagicMock(status_code=200, headers={})
-    page1.json.return_value = {"next": "tok", "features": []}
-    page2 = mock.MagicMock(status_code=200, headers={})
-    page2.json.return_value = {"next": None, "features": []}
-
-    client = mock.MagicMock(spec=requests.Session)
-    client.send.return_value = page1
-    client.request.return_value = page2
-
-    get_stats_data(
-        args={"monitoring_location_id": "USGS-1"},
-        service="observationNormals",
-        expand_percentiles=False,
-        client=client,
+    result = subprocess.run(
+        [sys.executable, "-c", code], capture_output=True, text=True
     )
-
-    geo = [m for m in _warning_messages(caplog) if "Geopandas not installed" in m]
-    assert len(geo) == 1
+    assert result.returncode == 0, result.stderr
+    assert result.stderr.count("Geopandas not installed") == 1
 
 
 def test_handle_stats_nesting_returns_empty_on_empty_features():