Skip to content

Commit b325f92

Browse files
thodson-usgsclaude
andcommitted
Tidy streamstats fix per /simplify review
Per /simplify review on PR #245: - Replace `json.loads(r.text)` with `r.json()` (matching the convention used in `nldi.py` and `nwis.py`) and drop the now-unused `import json` from `dataretrieval/streamstats.py`. - Remove the dead `format == "shape"` branch — a stale Fiona-stub (`# use Fiona to return a shape object` + `pass`) that silently fell through to `Watershed.from_streamstats_json(data)`. The docstring no longer mentions `"shape"` so removing the branch matches the contract. - Drop four trivial unit tests that each exercised a one-line behavior change obvious from the diff (`from_streamstats_json` returns an instance, `format="object"` returns a dict, `__init__` populates self, `_workspaceID` alias resolves). Keep the load-bearing one: `test_from_streamstats_json_does_not_mutate_class` guards against re-introducing the class-mutation antipattern under refactor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e1b424d commit b325f92

2 files changed

Lines changed: 2 additions & 44 deletions

File tree

dataretrieval/streamstats.py

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
66
"""
77

8-
import json
9-
108
import requests
119

1210

@@ -137,11 +135,7 @@ def get_watershed(
137135
if format == "geojson":
138136
return r
139137

140-
if format == "shape":
141-
# use Fiona to return a shape object
142-
pass
143-
144-
data = json.loads(r.text)
138+
data = r.json()
145139

146140
if format == "object":
147141
return data

tests/streamstats_test.py

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
"""Tests for the streamstats module."""
22

3-
import json
4-
5-
from dataretrieval.streamstats import Watershed, get_watershed
3+
from dataretrieval.streamstats import Watershed
64

75
SAMPLE_JSON = {
86
"featurecollection": [
@@ -14,12 +12,6 @@
1412
}
1513

1614

17-
def test_from_streamstats_json_returns_instance():
18-
"""Watershed.from_streamstats_json must return an instance, not the class."""
19-
w = Watershed.from_streamstats_json(SAMPLE_JSON)
20-
assert isinstance(w, Watershed)
21-
22-
2315
def test_from_streamstats_json_does_not_mutate_class():
2416
"""Two watersheds must not share state via class-level attributes."""
2517
other_json = {
@@ -39,31 +31,3 @@ def test_from_streamstats_json_does_not_mutate_class():
3931
assert w2.parameters[0]["code"] == "OTHER"
4032
assert w1.watershed_point["id"] == "pt-1"
4133
assert w2.watershed_point["id"] == "pt-2"
42-
43-
44-
def test_get_watershed_object_returns_dict(requests_mock):
45-
"""get_watershed(format='object') must return parsed JSON, not None."""
46-
url = "https://streamstats.usgs.gov/streamstatsservices/watershed.geojson"
47-
requests_mock.get(url, text=json.dumps(SAMPLE_JSON))
48-
49-
result = get_watershed("NY", -74.524, 43.939, format="object")
50-
assert isinstance(result, dict)
51-
assert result["workspaceID"] == "NY20240101000000000"
52-
53-
54-
def test_watershed_init_populates_instance(requests_mock):
55-
"""Watershed(...) must populate the instance (regression: previously discarded)."""
56-
url = "https://streamstats.usgs.gov/streamstatsservices/watershed.geojson"
57-
requests_mock.get(url, text=json.dumps(SAMPLE_JSON))
58-
59-
w = Watershed("NY", -74.524, 43.939)
60-
assert w.workspace_id == "NY20240101000000000"
61-
assert w.parameters[0]["code"] == "DRNAREA"
62-
assert w.watershed_point["id"] == "pt-1"
63-
assert w.watershed_polygon["id"] == "poly-1"
64-
65-
66-
def test_workspace_id_back_compat_alias():
67-
"""Legacy `_workspaceID` attribute should still resolve."""
68-
w = Watershed.from_streamstats_json(SAMPLE_JSON)
69-
assert w._workspaceID == w.workspace_id == "NY20240101000000000"

0 commit comments

Comments
 (0)