Skip to content

Commit 6baa4ab

Browse files
authored
fix(waterdata): attach EPSG:4326 CRS to OGC GeoDataFrames (#343)
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 <arpitjain099@gmail.com> * 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.
1 parent 74c4856 commit 6baa4ab

3 files changed

Lines changed: 63 additions & 2 deletions

File tree

dataretrieval/ogc/shaping.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@
3131

3232
logger = logging.getLogger(__name__)
3333

34+
# Water Data OGC coordinates are published in WGS84, so attach that CRS where the
35+
# GeoDataFrame is built (otherwise the result is CRS-naive and ``to_crs`` /
36+
# spatial joins fail). Mirrors the constants in ``nldi`` (EPSG:4326) and ``nwis``
37+
# (EPSG:4269).
38+
_CRS = "EPSG:4326"
39+
3440
# Whether geopandas is present is a static, environment-level fact, so warn
3541
# once here at import time rather than per query/chunk.
3642
if not GEOPANDAS:
@@ -141,7 +147,8 @@ def _get_resp_data(
141147
# a plain DataFrame. Features that already carry geometry (the common
142148
# sites case) are passed through without a per-feature dict copy.
143149
df = gpd.GeoDataFrame.from_features(
144-
[f if "geometry" in f else {**f, "geometry": None} for f in features]
150+
[f if "geometry" in f else {**f, "geometry": None} for f in features],
151+
crs=_CRS,
145152
)
146153
# Mirror the non-geopandas branch's defensive ``f.get("id")`` so a feature
147154
# missing a top-level ``id`` yields None rather than a KeyError.

dataretrieval/waterdata/stats.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
_run_sync,
2323
)
2424
from dataretrieval.ogc.shaping import (
25+
_CRS,
2526
GEOPANDAS,
2627
_attach_coordinates,
2728
_empty_feature_frame,
@@ -111,7 +112,8 @@ def _handle_nesting(
111112
# can't ``KeyError`` on a stats feature that omits geometry — mirrors
112113
# the guard in :func:`engine._get_resp_data`.
113114
df = gpd.GeoDataFrame.from_features(
114-
[f if "geometry" in f else {**f, "geometry": None} for f in features]
115+
[f if "geometry" in f else {**f, "geometry": None} for f in features],
116+
crs=_CRS,
115117
).drop(columns=["data"], errors="ignore")
116118

117119
# Unnest json features, properties, data, and values while retaining necessary

tests/waterdata_utils_test.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,58 @@ class _Sentinel:
596596
assert isinstance(result, _Sentinel)
597597

598598

599+
def test_get_resp_data_attaches_wgs84_crs():
600+
"""A geometry-bearing Water Data page should come back tagged as
601+
EPSG:4326 (the CRS the coordinates are published in), so callers can
602+
run ``to_crs`` / spatial joins without first patching in a CRS by
603+
hand. Regression for the ``.crs is None`` reported in issue #342."""
604+
geopandas = pytest.importorskip("geopandas")
605+
606+
resp = _resp_ok(
607+
[
608+
{
609+
"type": "Feature",
610+
"id": "USGS-01",
611+
"geometry": {"type": "Point", "coordinates": [-76.5, 39.2]},
612+
"properties": {"monitoring_location_id": "USGS-01"},
613+
}
614+
]
615+
)
616+
df = _get_resp_data(resp, geopd=True)
617+
assert isinstance(df, geopandas.GeoDataFrame)
618+
assert df.crs == "EPSG:4326"
619+
620+
621+
def test_handle_nesting_attaches_wgs84_crs():
622+
"""The stats path builds its GeoDataFrame the same way, so it should
623+
carry EPSG:4326 too (issue #342)."""
624+
geopandas = pytest.importorskip("geopandas")
625+
626+
body = {
627+
"next": None,
628+
"features": [
629+
{
630+
"type": "Feature",
631+
"geometry": {"type": "Point", "coordinates": [-76.5, 39.2]},
632+
"properties": {
633+
"monitoring_location_id": "USGS-01",
634+
"data": [
635+
{
636+
"parameter_code": "00060",
637+
"unit_of_measure": "ft^3/s",
638+
"parent_time_series_id": "ts-1",
639+
"values": [{"statistic_id": "mean", "value": 10.0}],
640+
}
641+
],
642+
},
643+
}
644+
],
645+
}
646+
df = _handle_nesting(body, geopd=True)
647+
assert isinstance(df, geopandas.GeoDataFrame)
648+
assert df.crs == "EPSG:4326"
649+
650+
599651
def test_handle_nesting_tolerates_missing_features_key():
600652
"""A 200 response with a body that doesn't carry ``features`` at
601653
all (rare but seen in error envelopes) must also short-circuit

0 commit comments

Comments
 (0)