Skip to content

Commit 42852a8

Browse files
committed
Centralize monitoring_location_id check in _get_args; trim narration
/simplify findings: 1. Move the per-function `monitoring_location_id = _check_monitoring_location_id(...)` into `_get_args` itself. Eliminates 13 copy-paste call sites in api.py and closes the bug class Copilot found twice (four functions had been missed when added piecemeal). New `get_*` functions inherit validation automatically. 2. Drop `_LIST_ONLY_STR_PARAMS` (a one-element `frozenset`). Inline the `properties` special case with a literal `elif k == "properties":` plus a one-line WHY comment. 3. Compress the three-paragraph narration in `_construct_api_requests` to a single line explaining the only non-obvious bit (`len()` instead of truthiness, because of numpy ndarray). 4. Add `begin_utc`/`end_utc` to `_DATE_RANGE_PARAMS`. `get_time_series_metadata` exposes both as range filters but the constant was missing them, so a two-element list would have been treated as a multi-value POST/CQL2 filter instead of being formatted as an ISO-8601 interval. 5. Drop the now-unused `_check_monitoring_location_id` import from api.py.
1 parent aa98d23 commit 42852a8

2 files changed

Lines changed: 8 additions & 29 deletions

File tree

dataretrieval/waterdata/api.py

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
)
2828
from dataretrieval.waterdata.utils import (
2929
SAMPLES_URL,
30-
_check_monitoring_location_id,
3130
_check_profiles,
3231
_default_headers,
3332
_get_args,
@@ -232,7 +231,6 @@ def get_daily(
232231
... last_modified="P7D",
233232
... )
234233
"""
235-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
236234
service = "daily"
237235
output_id = "daily_id"
238236

@@ -421,7 +419,6 @@ def get_continuous(
421419
... filter_lang="cql-text",
422420
... )
423421
"""
424-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
425422
service = "continuous"
426423
output_id = "continuous_id"
427424

@@ -720,7 +717,6 @@ def get_monitoring_locations(
720717
... properties=["monitoring_location_id", "state_name", "country_name"],
721718
... )
722719
"""
723-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
724720
service = "monitoring-locations"
725721
output_id = "monitoring_location_id"
726722

@@ -944,7 +940,6 @@ def get_time_series_metadata(
944940
... begin="1990-01-01/..",
945941
... )
946942
"""
947-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
948943
service = "time-series-metadata"
949944
output_id = "time_series_id"
950945

@@ -1181,7 +1176,6 @@ def get_combined_metadata(
11811176
service = "combined-metadata"
11821177
output_id = "combined_meta_id"
11831178

1184-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
11851179
args = _get_args(locals())
11861180

11871181
return get_ogc_data(args, output_id, service)
@@ -1373,7 +1367,6 @@ def get_latest_continuous(
13731367
... monitoring_location_id=["USGS-05114000", "USGS-09423350"]
13741368
... )
13751369
"""
1376-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
13771370
service = "latest-continuous"
13781371
output_id = "latest_continuous_id"
13791372

@@ -1570,7 +1563,6 @@ def get_latest_daily(
15701563
... monitoring_location_id=["USGS-05114000", "USGS-09423350"]
15711564
... )
15721565
"""
1573-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
15741566
service = "latest-daily"
15751567
output_id = "latest_daily_id"
15761568

@@ -1761,7 +1753,6 @@ def get_field_measurements(
17611753
... time="P20Y",
17621754
... )
17631755
"""
1764-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
17651756
service = "field-measurements"
17661757
output_id = "field_measurement_id"
17671758

@@ -1883,7 +1874,6 @@ def get_field_measurements_metadata(
18831874
service = "field-measurements-metadata"
18841875
output_id = "field_series_id"
18851876

1886-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
18871877
args = _get_args(locals())
18881878

18891879
return get_ogc_data(args, output_id, service)
@@ -2004,7 +1994,6 @@ def get_peaks(
20041994
service = "peaks"
20051995
output_id = "peak_id"
20061996

2007-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
20081997
args = _get_args(locals())
20091998

20101999
return get_ogc_data(args, output_id, service)
@@ -2536,7 +2525,6 @@ def get_stats_por(
25362525
... )
25372526
"""
25382527
# Build argument dictionary, omitting None values
2539-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
25402528
params = _get_args(locals(), exclude={"expand_percentiles"})
25412529

25422530
return get_stats_data(
@@ -2666,7 +2654,6 @@ def get_stats_date_range(
26662654
... )
26672655
"""
26682656
# Build argument dictionary, omitting None values
2669-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
26702657
params = _get_args(locals(), exclude={"expand_percentiles"})
26712658

26722659
return get_stats_data(
@@ -2839,7 +2826,6 @@ def get_channel(
28392826
... monitoring_location_id="USGS-02238500",
28402827
... )
28412828
"""
2842-
monitoring_location_id = _check_monitoring_location_id(monitoring_location_id)
28432829
service = "channel-measurements"
28442830
output_id = "channel_measurements_id"
28452831

dataretrieval/waterdata/utils.py

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,9 @@ def _switch_properties_id(properties: list[str] | None, id_name: str, service: s
149149
# string list. Used by ``_construct_api_requests`` to keep them out of the
150150
# POST/CQL2 multi-value path and to route them through ``_format_api_dates``,
151151
# and by ``_NO_NORMALIZE_PARAMS`` to bypass string-iterable normalization.
152-
_DATE_RANGE_PARAMS = frozenset({"datetime", "last_modified", "begin", "end", "time"})
152+
_DATE_RANGE_PARAMS = frozenset(
153+
{"datetime", "last_modified", "begin", "begin_utc", "end", "end_utc", "time"}
154+
)
153155

154156

155157
def _parse_datetime(value: str) -> datetime | None:
@@ -468,11 +470,7 @@ def _construct_api_requests(
468470
dates = service == "daily" and i != "last_modified"
469471
params[i] = _format_api_dates(params[i], date=dates)
470472

471-
# Join bbox/properties into the comma-separated form the OGC API expects.
472-
# For ``bbox`` use ``len() > 0`` so ``numpy.ndarray`` inputs don't trip
473-
# the ambiguous truth-value error; for ``properties`` the truthy check is
474-
# right because ``_get_args`` always materializes it to a list (and
475-
# ``_switch_properties_id`` further upstream returns ``[]`` for None).
473+
# `len()` instead of truthiness: a numpy ndarray would raise on `if bbox:`.
476474
if bbox is not None and len(bbox) > 0:
477475
params["bbox"] = ",".join(map(str, bbox))
478476
if properties:
@@ -1203,12 +1201,6 @@ def _check_profiles(
12031201
"thresholds",
12041202
}
12051203

1206-
# Param names that must be a list of strings (never a single string).
1207-
# A single string passed in would iterate as characters in
1208-
# ``_construct_api_requests``'s ``",".join(...)`` step, producing a
1209-
# malformed URL. ``_get_args`` wraps single-string input into a list.
1210-
_LIST_ONLY_STR_PARAMS = frozenset({"properties"})
1211-
12121204

12131205
def _normalize_str_iterable(
12141206
value: str | Iterable[str] | None,
@@ -1344,9 +1336,10 @@ def _get_args(
13441336
for k, v in local_vars.items():
13451337
if k in to_exclude or v is None:
13461338
continue
1347-
if k in _LIST_ONLY_STR_PARAMS:
1348-
# Wrap a single string so the downstream `",".join(...)` doesn't
1349-
# iterate it as characters.
1339+
if k == "monitoring_location_id":
1340+
args[k] = _check_monitoring_location_id(v)
1341+
elif k == "properties":
1342+
# `",".join(properties)` would iterate a bare string as characters.
13501343
args[k] = [v] if isinstance(v, str) else _normalize_str_iterable(v, k)
13511344
elif (
13521345
k in _NO_NORMALIZE_PARAMS

0 commit comments

Comments
 (0)