Skip to content

Commit f8d0a3f

Browse files
thodson-usgsclaude
andcommitted
Tidy nldi cleanup per /simplify review
Per /simplify review on PR #252: - Hoist `_VALID_NAVIGATION_MODES` to the module-level constants block alongside `NLDI_API_BASE_URL`/`_CRS` rather than wedging it between two `_validate_*` helpers. - Drop the redundant `have_latlong` local in `get_features`; inline `lat is not None` directly. Hoist the duplicated `err_msg = _features_err_msg(...)` to a single call after the comid/feature-source if/else branch. - Drop the unused `mock_request_data_sources` call in `test_get_flowlines_by_comid_includes_trim_start` — the comid path doesn't hit `_validate_data_source`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 86d5891 commit f8d0a3f

2 files changed

Lines changed: 15 additions & 23 deletions

File tree

dataretrieval/nldi.py

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
NLDI_API_BASE_URL = "https://api.water.usgs.gov/nldi/linked-data"
1414
_AVAILABLE_DATA_SOURCES = None
1515
_CRS = "EPSG:4326"
16+
_VALID_NAVIGATION_MODES = ("UM", "DM", "UT", "DD")
1617

1718

1819
def _query_nldi(url, query_params, error_message):
@@ -230,8 +231,7 @@ def get_features(
230231
if (lat is None) != (long is None):
231232
raise ValueError("Both lat and long are required")
232233

233-
have_latlong = lat is not None
234-
if have_latlong:
234+
if lat is not None:
235235
if comid is not None:
236236
raise ValueError(
237237
"Provide only one origin type - comid cannot be provided"
@@ -242,6 +242,9 @@ def get_features(
242242
"Provide only one origin type - feature_source and feature_id cannot"
243243
" be provided with lat or long"
244244
)
245+
url = f"{NLDI_API_BASE_URL}/comid/position"
246+
query_params = {"coords": f"POINT({long} {lat})"}
247+
err_msg = f"Error getting features for lat '{lat}' and long '{long}'"
245248
else:
246249
if (comid is not None or data_source is not None) and navigation_mode is None:
247250
raise ValueError(
@@ -254,24 +257,17 @@ def get_features(
254257
_validate_data_source(feature_source)
255258
if navigation_mode:
256259
navigation_mode = _validate_navigation_mode(navigation_mode)
257-
258-
if have_latlong:
259-
url = f"{NLDI_API_BASE_URL}/comid/position"
260-
query_params = {"coords": f"POINT({long} {lat})"}
261-
err_msg = f"Error getting features for lat '{lat}' and long '{long}'"
262-
elif navigation_mode:
263-
if feature_source:
264-
url = f"{NLDI_API_BASE_URL}/{feature_source}/{feature_id}/navigation"
260+
if feature_source:
261+
url = f"{NLDI_API_BASE_URL}/{feature_source}/{feature_id}/navigation"
262+
else:
263+
url = f"{NLDI_API_BASE_URL}/comid/{comid}/navigation"
264+
url += f"/{navigation_mode}/{data_source}"
265+
query_params = {"distance": str(distance)}
266+
if stop_comid is not None:
267+
query_params["stopComid"] = str(stop_comid)
265268
else:
266-
url = f"{NLDI_API_BASE_URL}/comid/{comid}/navigation"
267-
url += f"/{navigation_mode}/{data_source}"
268-
query_params = {"distance": str(distance)}
269-
if stop_comid is not None:
270-
query_params["stopComid"] = str(stop_comid)
271-
err_msg = _features_err_msg(feature_source, feature_id, comid, data_source)
272-
else:
273-
url = f"{NLDI_API_BASE_URL}/{feature_source}/{feature_id}"
274-
query_params = {}
269+
url = f"{NLDI_API_BASE_URL}/{feature_source}/{feature_id}"
270+
query_params = {}
275271
err_msg = _features_err_msg(feature_source, feature_id, comid, data_source)
276272

277273
feature_collection = _query_nldi(url, query_params, err_msg)
@@ -466,9 +462,6 @@ def _validate_data_source(data_source: str):
466462
raise ValueError(err_msg)
467463

468464

469-
_VALID_NAVIGATION_MODES = ("UM", "DM", "UT", "DD")
470-
471-
472465
def _features_err_msg(feature_source, feature_id, comid, data_source) -> str:
473466
if feature_source is not None:
474467
return (

tests/nldi_test.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,6 @@ def test_get_flowlines_by_comid_includes_trim_start(requests_mock):
308308
"""Regression: trim_start was previously dropped on the comid code path."""
309309
request_url = f"{NLDI_API_BASE_URL}/comid/13294314/navigation/UM/flowlines"
310310
response_file_path = "tests/data/nldi_get_flowlines_by_comid.json"
311-
mock_request_data_sources(requests_mock)
312311
mock_request(requests_mock, request_url, response_file_path)
313312

314313
get_flowlines(navigation_mode="UM", comid=13294314, trim_start=True)

0 commit comments

Comments
 (0)