Skip to content

fix(waterdata): attach EPSG:4326 CRS to OGC GeoDataFrames#343

Merged
thodson-usgs merged 2 commits into
DOI-USGS:mainfrom
arpitjain099:chore/waterdata-crs
Jul 7, 2026
Merged

fix(waterdata): attach EPSG:4326 CRS to OGC GeoDataFrames#343
thodson-usgs merged 2 commits into
DOI-USGS:mainfrom
arpitjain099:chore/waterdata-crs

Conversation

@arpitjain099

Copy link
Copy Markdown
Contributor

This fixes #342. The waterdata getters build their GeoDataFrame with gpd.GeoDataFrame.from_features(...) but never pass a crs, so the result comes back with .crs of None even though the coordinates are published in EPSG:4326 and the get_monitoring_locations docstring says as much. That leaves the frame CRS-naive, so to_crs and spatial joins raise on an otherwise valid result, and it's out of step with the nldi (EPSG:4326) and nwis (EPSG:4269) modules that already tag theirs.

I added a shared _CRS constant in ogc/shaping.py and pass it at both build sites (shaping and waterdata/stats), mirroring how nldi/nwis do it. I confirmed the fix with new offline regression tests that fail before the change (crs is None) and pass after (crs is EPSG:4326) for both the sites path and the stats path; the rest of the waterdata suite and ruff stay green. Thanks for taking a look.

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 DOI-USGS#342

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@thodson-usgs

Copy link
Copy Markdown
Collaborator

thanks @arpitjain099, @matinguku

@mikemahoney218-usgs, before I merge this, could you confirm whether or not EPSG:4326 is unversal. If not, we might assign the crs with a warning.

@mikemahoney218-usgs

mikemahoney218-usgs commented Jul 7, 2026

Copy link
Copy Markdown

Universal unless a user provides the crs query argument, in which case we use the CRS requested (currently only supports OGC's variants of NAD83 and NAD27). We mostly don't document that functionality because the way OGC handles CRS is needlessly complex, though, so I'd suspect ~100% of the time it's 4326

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) <noreply@anthropic.com>
@thodson-usgs thodson-usgs merged commit 6baa4ab into DOI-USGS:main Jul 7, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

waterdata GeoDataFrames are returned with .crs = None (should be EPSG:4326)

3 participants