From 42107f9fca3239172c6cde7f3d6a734d14cb3b3e Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Tue, 7 Jul 2026 09:32:42 +0900 Subject: [PATCH 1/2] fix(waterdata): attach EPSG:4326 CRS to OGC GeoDataFrames The waterdata getters build their GeoDataFrame via gpd.GeoDataFrame.from_features(...) without a crs= argument, so the returned object has .crs is None even though the coordinates are published in EPSG:4326 (WGS84) and the get_monitoring_locations docstring says so. That makes CRS-aware ops like to_crs and spatial joins fail on an otherwise valid result, and it's inconsistent with the nldi (EPSG:4326) and nwis (EPSG:4269) modules, which already tag their frames. Pass the documented CRS at both build sites (ogc/shaping.py and waterdata/stats.py) via a shared _CRS constant. Fixes #342 Signed-off-by: Arpit Jain --- dataretrieval/ogc/shaping.py | 9 +++++- dataretrieval/waterdata/stats.py | 4 ++- tests/waterdata_utils_test.py | 54 ++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/dataretrieval/ogc/shaping.py b/dataretrieval/ogc/shaping.py index b8e3c8e5..4df242f2 100644 --- a/dataretrieval/ogc/shaping.py +++ b/dataretrieval/ogc/shaping.py @@ -31,6 +31,12 @@ logger = logging.getLogger(__name__) +# Water Data OGC coordinates are published in WGS84, so attach that CRS where the +# GeoDataFrame is built (otherwise the result is CRS-naive and ``to_crs`` / +# spatial joins fail). Mirrors the constants in ``nldi`` (EPSG:4326) and ``nwis`` +# (EPSG:4269). +_CRS = "EPSG:4326" + # Whether geopandas is present is a static, environment-level fact, so warn # once here at import time rather than per query/chunk. if not GEOPANDAS: @@ -141,7 +147,8 @@ def _get_resp_data( # a plain DataFrame. Features that already carry geometry (the common # sites case) are passed through without a per-feature dict copy. df = gpd.GeoDataFrame.from_features( - [f if "geometry" in f else {**f, "geometry": None} for f in features] + [f if "geometry" in f else {**f, "geometry": None} for f in features], + crs=_CRS, ) # Mirror the non-geopandas branch's defensive ``f.get("id")`` so a feature # missing a top-level ``id`` yields None rather than a KeyError. diff --git a/dataretrieval/waterdata/stats.py b/dataretrieval/waterdata/stats.py index 5514bb50..2aa357de 100644 --- a/dataretrieval/waterdata/stats.py +++ b/dataretrieval/waterdata/stats.py @@ -22,6 +22,7 @@ _run_sync, ) from dataretrieval.ogc.shaping import ( + _CRS, GEOPANDAS, _attach_coordinates, _empty_feature_frame, @@ -111,7 +112,8 @@ def _handle_nesting( # can't ``KeyError`` on a stats feature that omits geometry — mirrors # the guard in :func:`engine._get_resp_data`. df = gpd.GeoDataFrame.from_features( - [f if "geometry" in f else {**f, "geometry": None} for f in features] + [f if "geometry" in f else {**f, "geometry": None} for f in features], + crs=_CRS, ).drop(columns=["data"], errors="ignore") # Unnest json features, properties, data, and values while retaining necessary diff --git a/tests/waterdata_utils_test.py b/tests/waterdata_utils_test.py index faf600bd..5c6f8d86 100644 --- a/tests/waterdata_utils_test.py +++ b/tests/waterdata_utils_test.py @@ -596,6 +596,60 @@ class _Sentinel: assert isinstance(result, _Sentinel) +def test_get_resp_data_attaches_wgs84_crs(): + """A geometry-bearing Water Data page should come back tagged as + EPSG:4326 (the CRS the coordinates are published in), so callers can + run ``to_crs`` / spatial joins without first patching in a CRS by + hand. Regression for the ``.crs is None`` reported in issue #342.""" + geopandas = pytest.importorskip("geopandas") + + resp = mock.MagicMock() + resp.json.return_value = { + "numberReturned": 1, + "features": [ + { + "type": "Feature", + "id": "USGS-01", + "geometry": {"type": "Point", "coordinates": [-76.5, 39.2]}, + "properties": {"monitoring_location_id": "USGS-01"}, + } + ], + } + df = _get_resp_data(resp, geopd=True) + assert isinstance(df, geopandas.GeoDataFrame) + assert df.crs == "EPSG:4326" + + +def test_handle_nesting_attaches_wgs84_crs(): + """The stats path builds its GeoDataFrame the same way, so it should + carry EPSG:4326 too (issue #342).""" + geopandas = pytest.importorskip("geopandas") + + body = { + "next": None, + "features": [ + { + "type": "Feature", + "geometry": {"type": "Point", "coordinates": [-76.5, 39.2]}, + "properties": { + "monitoring_location_id": "USGS-01", + "data": [ + { + "parameter_code": "00060", + "unit_of_measure": "ft^3/s", + "parent_time_series_id": "ts-1", + "values": [{"statistic_id": "mean", "value": 10.0}], + } + ], + }, + } + ], + } + df = _handle_nesting(body, geopd=True) + assert isinstance(df, geopandas.GeoDataFrame) + assert df.crs == "EPSG:4326" + + def test_handle_nesting_tolerates_missing_features_key(): """A 200 response with a body that doesn't carry ``features`` at all (rare but seen in error envelopes) must also short-circuit From c957d2d43bd6001d52d2ff1c3ca82ea1db838271 Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Tue, 7 Jul 2026 13:47:47 -0500 Subject: [PATCH 2/2] test(waterdata): reuse _resp_ok helper in CRS regression test Replace the hand-built mock.MagicMock response in test_get_resp_data_attaches_wgs84_crs with the file's existing _resp_ok() helper, which produces the same 200-OK feature payload. Drops duplicated mock scaffolding and matches the convention used by the other tests in this module. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/waterdata_utils_test.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/waterdata_utils_test.py b/tests/waterdata_utils_test.py index 5c6f8d86..95aabe30 100644 --- a/tests/waterdata_utils_test.py +++ b/tests/waterdata_utils_test.py @@ -603,18 +603,16 @@ def test_get_resp_data_attaches_wgs84_crs(): hand. Regression for the ``.crs is None`` reported in issue #342.""" geopandas = pytest.importorskip("geopandas") - resp = mock.MagicMock() - resp.json.return_value = { - "numberReturned": 1, - "features": [ + resp = _resp_ok( + [ { "type": "Feature", "id": "USGS-01", "geometry": {"type": "Point", "coordinates": [-76.5, 39.2]}, "properties": {"monitoring_location_id": "USGS-01"}, } - ], - } + ] + ) df = _get_resp_data(resp, geopd=True) assert isinstance(df, geopandas.GeoDataFrame) assert df.crs == "EPSG:4326"