Skip to content

Commit 5260a8f

Browse files
committed
Fix CI and reject Mapping inputs to _format_api_dates
Two issues: 1. CI failure on `test_get_args_basic` / `test_get_args_with_exclude`. The pre-existing tests used `monitoring_location_id="123"` purely as a placeholder to exercise `_get_args`'s filter/exclude logic. After the previous commit centralized the AGENCY-ID format check into `_get_args`, the bare "123" now raises ValueError before the dict-shaping assertions are reached. Update the placeholder to a well-formed "USGS-123" so the test's actual intent — filter + exclude wiring — still runs. 2. Copilot review: `_format_api_dates` silently accepts a Mapping by materializing its keys (`time={"2024-01-01": "x"}` → `["2024-01-01"]`), the same footgun `_normalize_str_iterable` already rejects elsewhere. Raise TypeError before the `list(...)` call. Adds a unit test.
1 parent 702ea29 commit 5260a8f

2 files changed

Lines changed: 20 additions & 4 deletions

File tree

dataretrieval/waterdata/utils.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,12 @@ def _format_api_dates(
241241
# Convert single string to list for uniform processing
242242
if isinstance(datetime_input, str):
243243
datetime_input = [datetime_input]
244+
elif isinstance(datetime_input, Mapping):
245+
# `list(mapping)` returns keys, which silently accepts the wrong shape.
246+
raise TypeError(
247+
f"date input must be a string or sequence of strings, "
248+
f"not {type(datetime_input).__name__}."
249+
)
244250
elif not isinstance(datetime_input, (list, tuple)):
245251
# Materialize any other iterable (pandas.Series, numpy.ndarray,
246252
# generator, ...) so the len()/subscript operations below work.

tests/waterdata_utils_test.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,26 @@
1515

1616
def test_get_args_basic():
1717
local_vars = {
18-
"monitoring_location_id": "123",
18+
"monitoring_location_id": "USGS-123",
1919
"service": "daily",
2020
"output_id": "daily_id",
2121
"none_val": None,
2222
"other": "val",
2323
}
2424
result = _get_args(local_vars)
25-
assert result == {"monitoring_location_id": "123", "other": "val"}
25+
assert result == {"monitoring_location_id": "USGS-123", "other": "val"}
2626

2727

2828
def test_get_args_with_exclude():
2929
local_vars = {
30-
"monitoring_location_id": "123",
30+
"monitoring_location_id": "USGS-123",
3131
"service": "daily",
3232
"output_id": "daily_id",
3333
"to_exclude": "secret",
3434
"other": "val",
3535
}
3636
result = _get_args(local_vars, exclude={"to_exclude"})
37-
assert result == {"monitoring_location_id": "123", "other": "val"}
37+
assert result == {"monitoring_location_id": "USGS-123", "other": "val"}
3838

3939

4040
def test_get_args_empty():
@@ -224,6 +224,16 @@ def test_format_api_dates_open_ended_range_with_none():
224224
assert _format_api_dates([None, "2024-01-01"], date=True) == "../2024-01-01"
225225

226226

227+
def test_format_api_dates_rejects_mapping():
228+
"""`time={"2024-01-01": "x"}` would silently materialize as the keys list,
229+
accepting input the user clearly didn't intend.
230+
"""
231+
import pytest
232+
233+
with pytest.raises(TypeError, match="date input must be a string or sequence"):
234+
_format_api_dates({"2024-01-01": "ignored"})
235+
236+
227237
def _make_response(status, body, reason=None, content_type="text/html"):
228238
resp = requests.Response()
229239
resp.status_code = status

0 commit comments

Comments
 (0)