Skip to content

Commit 426b82f

Browse files
thodson-usgsclaude
andcommitted
refactor(nldi): drop dead non-200 branch + its error_message plumbing
/simplify follow-up. After this PR routed nldi through query() -- which now raises a typed DataRetrievalError for every HTTP error response -- _query_nldi's `if status != 200` branch became unreachable for errors, and for the only status it could still see (a 2xx/3xx that query() passes through) it mislabeled a success as ClientError. Delete the branch plus the `error_message` parameter, the err_msg strings at the 5 call sites, and the `_features_err_msg` helper that existed only to feed it. Also fix a stale "414" -> "413/414" docstring in _raise_for_status. ruff clean; mypy --strict clean; 483 passed / 2 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5509be6 commit 426b82f

2 files changed

Lines changed: 9 additions & 46 deletions

File tree

dataretrieval/nldi.py

Lines changed: 8 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from json import JSONDecodeError
44
from typing import Any, Literal, cast
55

6-
from dataretrieval.exceptions import error_for_status
76
from dataretrieval.utils import query
87

98
try:
@@ -20,16 +19,11 @@
2019
def _query_nldi(
2120
url: str,
2221
query_params: dict[str, str],
23-
error_message: str,
2422
) -> dict[str, Any] | list[Any]:
25-
# A helper function to query the NLDI API
23+
# A helper function to query the NLDI API. ``query()`` already raises a
24+
# typed ``DataRetrievalError`` for any HTTP error response, so a returned
25+
# response is a success that we only need to parse.
2626
response = query(url, payload=query_params)
27-
if response.status_code != 200:
28-
raise error_for_status(
29-
response.status_code,
30-
f"{error_message} Error reason: {response.reason_phrase}",
31-
)
32-
3327
response_data: dict[str, Any] | list[Any] = {}
3428
try:
3529
response_data = response.json()
@@ -116,15 +110,7 @@ def get_flowlines(
116110
if stop_comid is not None:
117111
query_params["stopComid"] = str(stop_comid)
118112

119-
if feature_source:
120-
err_msg = (
121-
f"Error getting flowlines for feature source '{feature_source}'"
122-
f" and feature_id '{feature_id}'"
123-
)
124-
else:
125-
err_msg = f"Error getting flowlines for comid '{comid}'"
126-
127-
feature_collection = cast("dict[str, Any]", _query_nldi(url, query_params, err_msg))
113+
feature_collection = cast("dict[str, Any]", _query_nldi(url, query_params))
128114
if as_json:
129115
return feature_collection
130116
gdf = _features_to_gdf(feature_collection)
@@ -176,11 +162,7 @@ def get_basin(
176162
"simplified": simplified_str,
177163
"splitCatchment": split_catchment_str,
178164
}
179-
err_msg = (
180-
f"Error getting basin for feature source '{feature_source}' and "
181-
f"feature_id '{feature_id}'"
182-
)
183-
feature_collection = cast("dict[str, Any]", _query_nldi(url, query_params, err_msg))
165+
feature_collection = cast("dict[str, Any]", _query_nldi(url, query_params))
184166
if as_json:
185167
return feature_collection
186168
gdf = _features_to_gdf(feature_collection)
@@ -270,7 +252,6 @@ def get_features(
270252
)
271253
url = f"{NLDI_API_BASE_URL}/comid/position"
272254
query_params = {"coords": f"POINT({long} {lat})"}
273-
err_msg = f"Error getting features for lat '{lat}' and long '{long}'"
274255
else:
275256
if (comid is not None or data_source is not None) and navigation_mode is None:
276257
raise ValueError(
@@ -294,9 +275,8 @@ def get_features(
294275
else:
295276
url = f"{NLDI_API_BASE_URL}/{feature_source}/{feature_id}"
296277
query_params = {}
297-
err_msg = _features_err_msg(feature_source, feature_id, comid, data_source)
298278

299-
feature_collection = cast("dict[str, Any]", _query_nldi(url, query_params, err_msg))
279+
feature_collection = cast("dict[str, Any]", _query_nldi(url, query_params))
300280
if as_json:
301281
return feature_collection
302282
gdf = _features_to_gdf(feature_collection)
@@ -331,8 +311,7 @@ def get_features_by_data_source(data_source: str) -> gpd.GeoDataFrame:
331311
# validate the data source
332312
_validate_data_source(data_source)
333313
url = f"{NLDI_API_BASE_URL}/{data_source}"
334-
err_msg = f"Error getting features for data source '{data_source}'"
335-
feature_collection = cast("dict[str, Any]", _query_nldi(url, {}, err_msg))
314+
feature_collection = cast("dict[str, Any]", _query_nldi(url, {}))
336315
gdf = _features_to_gdf(feature_collection)
337316
return gdf
338317

@@ -481,9 +460,7 @@ def _validate_data_source(data_source: str) -> None:
481460
# get the available data/feature sources - if not already cached
482461
if _AVAILABLE_DATA_SOURCES is None:
483462
url = f"{NLDI_API_BASE_URL}/"
484-
available_data_sources = _query_nldi(
485-
url, {}, "Error getting available data sources"
486-
)
463+
available_data_sources = _query_nldi(url, {})
487464
if not isinstance(available_data_sources, list) or not all(
488465
isinstance(ds, dict) and "source" in ds for ds in available_data_sources
489466
):
@@ -502,20 +479,6 @@ def _validate_data_source(data_source: str) -> None:
502479
raise ValueError(err_msg)
503480

504481

505-
def _features_err_msg(
506-
feature_source: str | None,
507-
feature_id: str | None,
508-
comid: int | None,
509-
data_source: str | None,
510-
) -> str:
511-
if feature_source is not None:
512-
return (
513-
f"Error getting features for feature source '{feature_source}'"
514-
f" and feature_id '{feature_id}', and data source '{data_source}'"
515-
)
516-
return f"Error getting features for comid '{comid}' and data source '{data_source}'"
517-
518-
519482
def _validate_navigation_mode(navigation_mode: str | None) -> str:
520483
if navigation_mode is None:
521484
raise ValueError(

dataretrieval/utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ def _raise_for_status(response: httpx.Response) -> None:
293293
294294
Shared by the legacy :func:`query` path (and ``nadp`` / ``streamstats``).
295295
Delegates the status-to-type mapping to
296-
:func:`dataretrieval.exceptions.error_for_status`; a 414 keeps the richer
296+
:func:`dataretrieval.exceptions.error_for_status`; a 413/414 keeps the richer
297297
"split your query" guidance via :func:`_url_too_long_error`.
298298
"""
299299
status = response.status_code

0 commit comments

Comments
 (0)