diff --git a/NEWS.md b/NEWS.md index f4dbd033..43d89b9c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,5 @@ +**10/01/2026:** **Bug fix:** `waterdata.get_nearest_continuous` turned a missing target (`NaT`, `None`, `NaN`, or `""`) into a `(time >= 'nan' AND time <= 'nan')` clause and sent it to the service, including when only one entry of a list was missing. It now raises `ValueError` before any request, naming how many targets are missing and the position of the first. Targets pandas cannot parse now raise a `ValueError` that names `targets`, rather than pandas' message, which named no argument and suggested a `format=` the getter does not accept. **Bug fix:** the same getter's `window` was not validated: a missing window (`None`, `'NaT'`) failed with an `AttributeError` while the filter was built, and a negative one (`'-PT5M'`) inverted every bound, so the query matched nothing and returned an empty frame indistinguishable from a gap in the data. Both now raise `ValueError` naming `window`; a zero window remains an exact-match query. **Behavior change:** a bare-number `window` now raises `TypeError`; pandas read `window=450` as 450 nanoseconds, so it matched almost nothing. Pass a duration such as `window='PT450S'`. Numeric `targets` raise `TypeError` for the same reason: `targets=[1.5e9]` meant as epoch seconds was read as 1970-01-01T00:00:01.5Z. **Behavior change:** calls that previously sent a malformed filter now fail locally, so a caller passing targets with gaps must drop or fill them first. + **09/09/2026:** **Bug fix:** code and identifier columns keep their leading zeros. A bare `pandas.read_csv` infers a zero-padded code as a number, so `waterdata.get_samples()` returned parameter code `00060` as `60` and HUC12 `070700050502` as `70700050502`, and `nwis.get_info()` returned `huc_cd` `02060005` as `2060005`. One rule now decides what a code column is — a name ending in `code`, the RDB abbreviation `_cd`, or a name containing `identifier`, `huc`, or `fips` — and every delimited response is parsed through it: the Samples and WQP CSV readers, `rdb.read_rdb` (which reads the names from the RDB header rather than the caller listing them), and the Water Use CSV pages. **Behavior change:** these columns now hold strings. `waterdata.get_samples()`: `USGSpcode`, `Location_HUCEightDigitCode`, `Location_HUCTwelveDigitCode`, `SampleCollectionMethod_Identifier` (`get_samples_summary()` shares the parse; no column in its current profile was affected). `nwis.get_info()`, `nwis.what_sites()`, and `nwis.get_record(service="site")`: `huc_cd`, `state_cd`, `county_cd`, `district_cd`. A comparison against a number — `df["USGSpcode"] == 60` — or a merge onto a numeric key now matches nothing instead of raising, so compare against the padded string (`== "00060"`) or call `.astype(int)` where the number is what you want. **Behavior change:** a count whose name reads as an identifier is numeric again. WQP's `AlternateLocation_IdentifierCount` has been read as text since 05/31/2026 because "Identifier" appears in its name; a name ending in `count` is now excluded from the rule, so the same column has one dtype in every service that reports it. Measurement columns are unchanged, and the `waterdata` OGC getters and `ngwmn` were never affected: their JSON responses deliver codes as strings and numeric coercion there is limited to a fixed list of measurement columns. **Correction to the 1.2.0 notes:** the same fix was applied to the nine `wqp` getters on 05/31/2026 and never recorded here — `wqp.get_results()` and the `what_*` getters have returned HUCs, parameter codes, and FIPS codes as strings since that release. **09/01/2026:** **Announcement:** We at USGS Water Data for the Nation want your feedback! Tell us how we're doing by taking our quick [survey](https://usgswaterresources.gov1.qualtrics.com/jfe/form/SV_07gX8G1DeOtVrH8), available through September 2026. diff --git a/dataretrieval/waterdata/nearest.py b/dataretrieval/waterdata/nearest.py index 666d57c3..074d02cf 100644 --- a/dataretrieval/waterdata/nearest.py +++ b/dataretrieval/waterdata/nearest.py @@ -26,6 +26,12 @@ OnTie = Literal["first", "last", "mean"] _VALID_ON_TIE: tuple[OnTie, ...] = get_args(OnTie) +# ``pandas.api.types.infer_dtype`` results that ``pandas.to_datetime`` would +# read as epoch nanoseconds rather than reject. +_NUMERIC_INFERRED_DTYPES = frozenset( + {"integer", "floating", "mixed-integer-float", "decimal"} +) + class _ResumableCall(Protocol): """Structural subset of a fan-out call needed by the outer decorator.""" @@ -128,22 +134,27 @@ def get_nearest_continuous( targets : list-like of datetime-convertible Target timestamps. Naive datetimes are treated as UTC. Accepts a list, ``pandas.Series``, ``pandas.DatetimeIndex``, ``numpy.ndarray``, - or anything ``pandas.to_datetime`` accepts. + or anything ``pandas.to_datetime`` accepts except numbers, which + it would read as epoch nanoseconds. Must contain at least one + timestamp. Missing timestamps (``NaT``, ``None``, ``NaN``, or + ``""``) are rejected. monitoring_location_id : string or iterable of strings, optional Forwarded to ``get_continuous``. parameter_code : string or iterable of strings, optional Forwarded to ``get_continuous``. window : string or ``pandas.Timedelta``, default ``"PT7M30S"`` - Half-window around each target, as an ISO 8601 duration - (``"PT7M30S"``, ``"PT15M"``, ``"PT1H"``, etc.). Also accepts - any other form ``pandas.Timedelta`` parses — ``HH:MM:SS`` - (``"00:07:30"``), pandas shorthand (``"7min30s"``, + How far before and after each target to search, as an ISO 8601 + duration (``"PT7M30S"``, ``"PT15M"``, ``"PT1H"``, etc.); an + observation matches if it lies within ``window`` of the target. + Also accepts any other form ``pandas.Timedelta`` parses — + ``HH:MM:SS`` (``"00:07:30"``), pandas shorthand (``"7min30s"``, ``"450s"``), or a ``pd.Timedelta`` directly. See the `pandas.Timedelta docs `_ for the full grammar. - Must be small enough that every target's window contains + Must not be negative; zero matches only an observation at exactly + the target time. Must be small enough that every target's window contains roughly one observation at the service cadence. The default matches a 15-minute continuous gage; widen (e.g. ``"PT15M"``) for irregular cadences or tolerance of data gaps. @@ -180,6 +191,13 @@ def get_nearest_continuous( Raises ------ + ValueError + If ``targets`` is empty, unparseable, or contains missing timestamps; + if ``window`` is unparseable, missing, or negative; or if ``on_tie`` + is not one of its options. + TypeError + If ``targets`` or ``window`` is a bare number, or if ``time``, ``filter``, or + ``filter_lang`` is passed in ``kwargs``. FanOutInterrupted If the underlying fan-out is interrupted. ``partial_frame`` and ``call.partial_frame`` contain nearest-selected rows with @@ -233,14 +251,7 @@ def get_nearest_continuous( """ _check_nearest_kwargs(kwargs, on_tie) target_index = _coerce_targets(targets) - window_td = pd.Timedelta(window) - - if len(target_index) == 0: - raise ValueError( - "targets is empty; there is nothing to find a nearest value for. " - "Pass at least one timestamp, e.g. targets=['2024-01-01 12:00'] " - "or a pandas DatetimeIndex." - ) + window_td = _coerce_window(window) selector = _NearestSelector(target_index, window_td, on_tie) filter_expr = _build_window_or_filter(target_index, window_td) @@ -316,11 +327,118 @@ def _coerce_targets(targets: Any) -> pd.DatetimeIndex: A bare scalar (string, ``Timestamp``, ``datetime``, …) becomes a one-element ``DatetimeIndex``; an iterable (list, ``Series``, ``ndarray``) is wrapped directly so its elements are preserved. + + Raises ``TypeError`` for numbers, which pandas reads as nanoseconds since + the epoch, and ``ValueError`` for an unparseable, empty, or missing target: + a missing target would be rendered as a ``'nan'`` CQL bound rather than a + window. + """ + values = targets if pd.api.types.is_list_like(targets) else [targets] + _reject_numeric_targets(values) + try: + parsed = pd.to_datetime(values, utc=True) + except (ValueError, TypeError) as exc: + # pandas' own message goes on to suggest ``format=``, which this getter + # does not accept; keep only the part that names the offending value. + detail = str(exc).splitlines()[0].split(". You might want to try")[0] + detail = detail.rstrip(".") + raise ValueError( + f"targets could not be parsed as timestamps: {detail}. Pass " + "timestamps in one consistent format, e.g. ISO 8601 " + "'2024-01-01T12:00:00Z', or parse them first with " + "pandas.to_datetime(..., format=...)." + ) from exc + index = pd.DatetimeIndex(parsed) + if len(index) == 0: + raise ValueError( + "targets is empty; there is nothing to find a nearest value for. " + "Pass at least one timestamp, e.g. targets=['2024-01-01 12:00'] " + "or a pandas DatetimeIndex." + ) + if index.hasnans: + raise ValueError(_missing_targets_message(index)) + return index + + +def _reject_numeric_targets(values: Any) -> None: + """Refuse numbers, which ``pandas.to_datetime`` reads as epoch nanoseconds. + + ``targets=[1.5e9]`` meant as epoch seconds would otherwise become + 1970-01-01T00:00:01.5Z and match nothing. Missing values are skipped, so a + lone ``NaN`` is still reported as a missing target. *values* is list-like. """ - parsed = pd.to_datetime(targets, utc=True) - if pd.api.types.is_scalar(parsed): - parsed = [parsed] - return pd.DatetimeIndex(parsed) + if pd.api.types.infer_dtype(values, skipna=True) in _NUMERIC_INFERRED_DTYPES: + # A numeric inference means at least one non-missing value exists. + # Formatted with str() so a numpy scalar reads as 1500000000.0, not + # np.float64(1500000000.0). + first = next(value for value in values if pd.notna(value)) + raise TypeError( + "targets must be timestamps, not numbers, which pandas would read " + f"as nanoseconds since 1970-01-01 (got {first}). Convert them first " + "with pandas.to_datetime(targets, unit=...), setting unit to what " + "your numbers count: 's', 'ms', 'us', or 'ns'." + ) + + +def _missing_targets_message(index: pd.DatetimeIndex) -> str: + """Describe the missing entries of *index* so a caller can find them.""" + missing = index.isna() + count = int(missing.sum()) + first = int(missing.argmax()) + if count == len(index): + entries = "its only entry is" if count == 1 else f"all {count} entries are" + return ( + f"targets has no valid timestamps; {entries} missing " + "(NaT, None, NaN, or ''). Pass at least one timestamp, e.g. " + "targets=['2024-01-01 12:00']." + ) + if count == 1: + found = f"1 missing timestamp (NaT, None, NaN, or ''), at position {first}" + else: + found = ( + f"{count} missing timestamps (NaT, None, NaN, or ''); the first is " + f"at position {first}" + ) + return ( + f"targets contains {found}, counting from 0. Remove those entries or " + "replace them with valid timestamps, e.g. targets.dropna() for a " + "pandas Series or DatetimeIndex." + ) + + +def _coerce_window(window: str | pd.Timedelta) -> pd.Timedelta: + """Parse ``window`` and reject the values that cannot bound a match. + + A bare number is refused because ``pandas.Timedelta`` reads it as + nanoseconds, so ``window=450`` meant as seconds would match nothing. A + missing window cannot be subtracted from the targets, and a negative one + inverts every bound; both would otherwise return an empty frame + indistinguishable from a gap in the data. + """ + example = "Pass an ISO 8601 duration, e.g. window='PT7M30S'." + if isinstance(window, (int, float)): + raise TypeError( + "window must be a duration string or Timedelta, not a number, " + f"which pandas would read as nanoseconds (got {window!r}). {example}" + ) + try: + window_td = pd.Timedelta(window) + except ValueError as exc: + raise ValueError( + f"window could not be parsed as a duration (got {window!r}). {example}" + ) from exc + if pd.isna(window_td): + raise ValueError(f"window is missing (got {window!r}). {example}") + if window_td < pd.Timedelta(0): + # ``window`` spans both sides of each target, so the sign carries no + # meaning; the magnitude is what the caller wants. + magnitude = (-window_td).isoformat() + raise ValueError( + "window must not be negative; it already " + "extends both before and after each target. Drop the sign, e.g. " + f"window={magnitude!r}." + ) + return window_td def _check_nearest_kwargs(kwargs: dict[str, Any], on_tie: OnTie) -> None: @@ -328,8 +446,9 @@ def _check_nearest_kwargs(kwargs: dict[str, Any], on_tie: OnTie) -> None: for forbidden in ("time", "filter", "filter_lang"): if forbidden in kwargs: raise TypeError( - f"get_nearest_continuous constructs its own {forbidden!r}; " - "do not pass it directly" + f"get_nearest_continuous sets {forbidden} itself. Remove " + f"{forbidden} from the call; choose the times with targets and " + "window." ) require_one_of(on_tie, _VALID_ON_TIE, name="on_tie") diff --git a/tests/waterdata_nearest_test.py b/tests/waterdata_nearest_test.py index 45d83418..4f0102a5 100644 --- a/tests/waterdata_nearest_test.py +++ b/tests/waterdata_nearest_test.py @@ -6,6 +6,7 @@ from unittest import mock +import numpy as np import pandas as pd import pytest @@ -13,6 +14,8 @@ from dataretrieval.interruptions import QuotaExhausted, ServiceInterrupted from dataretrieval.waterdata.nearest import get_nearest_continuous +_SITE = "USGS-02238500" + def _fake_df(rows): """Build a minimal DataFrame with the continuous-response columns.""" @@ -173,14 +176,210 @@ def test_multi_site_returns_row_per_target_per_site(patch_get_continuous): assert set(result["monitoring_location_id"]) == {"USGS-1", "USGS-2"} +# --- targets validation ------------------------------------------------------ +# Every rejection happens before ``get_continuous`` is called, so each test also +# asserts no request was made. Where a message suggests a fix, a companion test +# applies that fix literally and checks the call then succeeds. + + def test_empty_targets_raises(patch_get_continuous): """An empty ``targets`` is a call with no useful work to do and almost always a caller bug — raise rather than issue a request that returns nothing.""" with pytest.raises(ValueError, match="targets"): - get_nearest_continuous([], monitoring_location_id="USGS-02238500") + get_nearest_continuous([], monitoring_location_id=_SITE) + patch_get_continuous.assert_not_called() + + +_LONE_MISSING_TARGETS = { + "nat": pd.NaT, + "none": None, + "empty-string": "", + "float-nan": float("nan"), + "numpy-nat": pd.NaT.to_datetime64(), + "one-element-list": [None], +} + + +@pytest.mark.parametrize( + ("targets", "message"), + [(t, "its only entry is missing") for t in _LONE_MISSING_TARGETS.values()] + + [([None, pd.NaT, float("nan")], "all 3 entries are missing")], + ids=[*_LONE_MISSING_TARGETS, "all-of-three"], +) +def test_all_missing_targets_ask_for_a_timestamp( + patch_get_continuous, targets, message +): + """Removing every entry would leave nothing, so ask for a timestamp rather + than suggesting the caller remove them.""" + with pytest.raises(ValueError, match=message) as exc_info: + get_nearest_continuous(targets, monitoring_location_id=_SITE) + assert "Remove" not in str(exc_info.value) + patch_get_continuous.assert_not_called() + + +def test_missing_target_message_wording(patch_get_continuous): + """The full message for the common case: one gap in an otherwise valid list. + Missing timestamps must not become ``'nan'`` CQL bounds.""" + with pytest.raises(ValueError) as exc_info: + get_nearest_continuous( + ["2024-01-01T12:00:00Z", None], monitoring_location_id=_SITE + ) + assert str(exc_info.value) == ( + "targets contains 1 missing timestamp (NaT, None, NaN, or ''), at " + "position 1, counting from 0. Remove those entries or replace them with " + "valid timestamps, e.g. targets.dropna() for a pandas Series or " + "DatetimeIndex." + ) + patch_get_continuous.assert_not_called() + + +@pytest.mark.parametrize( + ("targets", "located"), + [ + ( + pd.Series([pd.Timestamp("2024-01-01T12:00:00Z"), pd.NaT]), + "1 missing timestamp (NaT, None, NaN, or ''), at position 1", + ), + ( + pd.DatetimeIndex(["2024-01-01T12:00:00Z", pd.NaT]), + "1 missing timestamp (NaT, None, NaN, or ''), at position 1", + ), + ( + pd.Series(["2024-01-01", float("nan"), "2024-01-03", ""]), + "2 missing timestamps (NaT, None, NaN, or ''); the first is at position 1", + ), + ], + ids=["series", "index", "series-nan-and-empty-string"], +) +def test_missing_target_entries_are_located(patch_get_continuous, targets, located): + """A long target list is only correctable if the message says where to look, + whatever container the targets arrive in.""" + with pytest.raises(ValueError) as exc_info: + get_nearest_continuous(targets, monitoring_location_id=_SITE) + assert located in str(exc_info.value) + patch_get_continuous.assert_not_called() + + +def test_missing_target_remedy_works_when_followed(patch_get_continuous): + """The suggested ``dropna()`` must produce a call that succeeds.""" + patch_get_continuous.return_value = (_fake_df([]), mock.Mock()) + targets = pd.Series(["2024-01-01T12:00:00Z", None]) + with pytest.raises(ValueError, match=r"targets\.dropna\(\)"): + get_nearest_continuous(targets, monitoring_location_id=_SITE) + get_nearest_continuous(targets.dropna(), monitoring_location_id=_SITE) + patch_get_continuous.assert_called_once() + + +@pytest.mark.parametrize( + ("targets", "shown"), + [ + (1.5e9, "1500000000.0"), + ([1.5e9, 1.6e9], "1500000000.0"), + (np.array([1, 2]), "1"), + (pd.Series([np.nan, 1.5e9]), "1500000000.0"), + ], + ids=["scalar", "list", "int-array", "series-leading-nan"], +) +def test_numeric_targets_are_refused(patch_get_continuous, targets, shown): + """``pandas.to_datetime(1.5e9)`` is 1970-01-01T00:00:01.5Z, never what was + meant by an epoch value. The message shows the first non-missing number.""" + with pytest.raises(TypeError, match="targets must be timestamps") as exc_info: + get_nearest_continuous(targets, monitoring_location_id=_SITE) + assert f"(got {shown})" in str(exc_info.value) + patch_get_continuous.assert_not_called() + + +@pytest.mark.parametrize( + ("unit", "number"), + [("s", 1.5e9), ("ms", 1.5e12), ("us", 1.5e15), ("ns", 1.5e18)], + ids=["s", "ms", "us", "ns"], +) +def test_numeric_targets_remedy_works_for_every_listed_unit( + patch_get_continuous, unit, number +): + """Each unit the message lists must convert to the intended timestamp.""" + patch_get_continuous.return_value = (_fake_df([]), mock.Mock()) + get_nearest_continuous( + pd.to_datetime([number], unit=unit), monitoring_location_id=_SITE + ) + assert "2017-07-14T02:32:30Z" in patch_get_continuous.call_args.kwargs["filter"] + + +@pytest.mark.parametrize( + "targets", + [["2024-01-01", "not a date"], ["2024-01-01", "2024-01-01T12:00Z"]], + ids=["garbage", "mixed-iso-forms"], +) +def test_unparseable_targets_name_the_argument(patch_get_continuous, targets): + """pandas' own message names no argument and suggests a ``format=`` this + getter does not accept; the rewrapped one names ``targets`` instead.""" + with pytest.raises(ValueError, match="targets could not be parsed") as exc_info: + get_nearest_continuous(targets, monitoring_location_id=_SITE) + assert "You might want to try" not in str(exc_info.value) + patch_get_continuous.assert_not_called() + + +# --- window validation ------------------------------------------------------- + + +def _call_with_window(window): + """Call the getter with one valid target, varying only ``window``.""" + return get_nearest_continuous( + ["2024-01-01T12:00:00Z"], monitoring_location_id=_SITE, window=window + ) + + +@pytest.mark.parametrize( + ("window", "message"), + [ + (None, r"window is missing \(got None\)"), + ("NaT", r"window is missing \(got 'NaT'\)"), + ("seven minutes", r"window could not be parsed as a duration"), + ("-PT5M", r"window must not be negative.*window='P0DT0H5M0S'"), + ( + pd.Timedelta(minutes=-7, seconds=-30), + r"window must not be negative.*window='P0DT0H7M30S'", + ), + ], + ids=["none", "nat", "garbage", "negative-string", "negative-timedelta"], +) +def test_unusable_window_raises_before_query(patch_get_continuous, window, message): + """A missing window crashed while building the filter; a negative one + inverted every bound and returned an empty frame as if no data existed.""" + with pytest.raises(ValueError, match=message): + _call_with_window(window) patch_get_continuous.assert_not_called() +@pytest.mark.parametrize("window", [450, 7.5], ids=["int", "float"]) +def test_numeric_window_is_refused(patch_get_continuous, window): + """``pandas.Timedelta(450)`` is 450 nanoseconds, never what was meant.""" + with pytest.raises(TypeError, match="window must be a duration.*nanoseconds"): + _call_with_window(window) + patch_get_continuous.assert_not_called() + + +@pytest.mark.parametrize( + ("window", "lower", "upper"), + [ + # The fix the negative-window message suggests: same span, sign dropped. + ("P0DT0H5M0S", "2024-01-01T11:55:00Z", "2024-01-01T12:05:00Z"), + # Zero is a legitimate degenerate window: an exact-match query. + ("PT0S", "2024-01-01T12:00:00Z", "2024-01-01T12:00:00Z"), + ], + ids=["negative-window-remedy", "zero"], +) +def test_accepted_window_bounds(patch_get_continuous, window, lower, upper): + patch_get_continuous.return_value = (_fake_df([]), mock.Mock()) + _call_with_window(window) + assert patch_get_continuous.call_args.kwargs["filter"] == ( + f"(time >= '{lower}' AND time <= '{upper}')" + ) + + +# --- other arguments and behavior -------------------------------------------- + + def test_rejects_time_kwarg(patch_get_continuous): with pytest.raises(TypeError, match="time"): get_nearest_continuous( @@ -241,9 +440,9 @@ def test_accepts_list_of_strings(patch_get_continuous): ], ) def test_window_accepts_any_pandas_timedelta_form(patch_get_continuous, window): - """Every representation ``pandas.Timedelta`` parses must produce the - same CQL filter. Documents the public contract: ``window`` is - whatever ``pd.Timedelta(window)`` returns.""" + """Every string or ``Timedelta`` form ``pandas.Timedelta`` parses must + produce the same CQL filter. Documents the public contract; bare numbers + and negative or missing windows are rejected (see window validation).""" targets = pd.to_datetime(["2023-06-15T10:30:00Z"], utc=True) patch_get_continuous.return_value = (_fake_df([]), mock.Mock())