diff --git a/NEWS.md b/NEWS.md index f683c9d2..2fa2bea4 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,5 @@ +**10/09/2026:** **Behavior change:** a date range passed as one `"start/end"` string, such as `time="2024-01-01T10:00:00/.."`, is now formatted like the list form `["2024-01-01T10:00:00", None]`, in the `waterdata` and `ngwmn` OGC getters and `waterdata.get_ratings()`. **Bug fix:** the string used to be sent unchanged, so the two spellings could select different data: a naive time was not converted from local time to UTC, an offset was not converted to `Z`, and a date-only collection such as `daily` received times. A side that is not a date (`time="2024-01-01/tomorrow"`) now raises `ValueError` before any request is sent, as does a string with more than one `/` or with a duration as either side (`"2024-01-01/P7D"`), which the services answered with HTTP 400. An empty side is an open bound, and `"../.."` means no date filter. A lone duration such as `"P7D"` is unchanged. **Bug fix:** a range with an open start, such as `[None, "2016-01-01"]` or `"../2016-01-01"`, failed with HTTP 403 in `ngwmn.get_water_level()` and `waterdata.get_ratings()`, because those services' firewalls reject `../`. Both now send an empty start (`"/2016-01-01"`), which both services read as open. The other Water Data getters still send `..`, because that service rejects an empty start. + **10/07/2026:** **Behavior change:** a date argument the package cannot read as a date -- a typo such as `time="2024-13-45"`, an unsupported format such as `"Jan 1 2024"`, or one bad end of a range such as `time=["2024-01-01", "tomorrow"]` -- now raises `ValueError` naming the argument and the value, before any request is sent. It used to drop the date filter: `waterdata.get_ratings()` searched with no `datetime` and silently returned every rating, and the OGC getters (`get_daily()`, `get_continuous()`, `ngwmn.get_water_level()`, and every `time`, `begin`, `end`, `datetime`, and `last_modified` argument, plus the deprecated `begin_utc` and `end_utc`) sent an empty `time=` that the service rejected with HTTP 400 "Invalid datetime format", which named no argument. To leave one end of a range open, pass `None` (`["2024-01-01", None]`) or `".."`; the getter docstrings now show this list form. A range whose ends are all open still means no date filter. **10/07/2026:** **Bug fix:** a `".."` endpoint in a two-value date range -- `time=["2024-01-01", ".."]`, the open-ended form shown in the `waterdata.get_ratings()` docstring -- discarded the whole range, because `..` was not recognized as an open bound and failed to parse as a date. `waterdata.get_ratings()` then searched with no `datetime` at all and returned every rating regardless of date (59 instead of 44 for the docstring's bounding box from 2026-09-20 on), and the OGC getters (`get_daily()`, `get_continuous()`, `get_field_measurements()`, and the other `time`, `begin`, `end`, and `last_modified` arguments) sent an empty `time=` that the service rejected with HTTP 400 "Invalid datetime format". `".."` is now an open bound like `None`, so `["2024-01-01", ".."]` sends the same range as `"2024-01-01/.."` and `["2024-01-01", None]`. diff --git a/dataretrieval/ngwmn.py b/dataretrieval/ngwmn.py index 7fc351ba..a5e3d998 100644 --- a/dataretrieval/ngwmn.py +++ b/dataretrieval/ngwmn.py @@ -80,10 +80,12 @@ # (there is no service-specific id name as there is for the main collections). _NGWMN_OUTPUT_ID = "id" -# NGWMN's request shape matches the generic OGC default (no CQL2-only or -# date-only collections), but its result columns need their own coercion and -# sort vocabulary: water-level observations are timestamped by ``sample_time`` -# (not the Water Data ``time``) and report depths/levels in feet. +# NGWMN has no CQL2-only or date-only collections, but its result columns need +# their own coercion and sort vocabulary: water-level observations are +# timestamped by ``sample_time`` (not the Water Data ``time``) and report +# depths/levels in feet. Its firewall answers ``datetime=../2016-01-01`` with +# HTTP 403, so an open start is sent empty (``/2016-01-01``), which it reads as +# open. NGWMN_DIALECT = OgcDialect( time_cols=frozenset({"sample_time"}), numerical_cols=frozenset( @@ -94,6 +96,7 @@ } ), sort_cols=("sample_time", "monitoring_location_id"), + open_start="", ) diff --git a/dataretrieval/ogc/dates.py b/dataretrieval/ogc/dates.py index 0fe5cd9e..3e6c2236 100644 --- a/dataretrieval/ogc/dates.py +++ b/dataretrieval/ogc/dates.py @@ -115,9 +115,27 @@ def _coerce_to_list( return list(datetime_input) -def _is_passthrough(single: str) -> bool: - """True when a single-element input should be returned as-is.""" - return bool(_DURATION_RE.match(single) or "/" in single) +def _split_interval(interval: str, *, name: str) -> tuple[str, str]: + """Split a ``"start/end"`` string into its two sides. + + Each side is then formatted like a list element, so + ``"2024-01-01T10:00:00/.."`` sends the same range as + ``["2024-01-01T10:00:00", None]``. + + Raises ``ValueError`` naming *name* unless the string is two sides + separated by one ``"/"``, neither of them a duration: the Water Data and + NGWMN services answer ``"2024-01-01/P7D"`` and ``"P7D/2024-01-08"`` with + HTTP 400. + """ + sides = interval.split("/") + if len(sides) != 2 or any(_DURATION_RE.match(side) for side in sides): + raise ValueError( + f"{name} is not a valid interval: {interval!r}. Pass a start and " + "an end separated by '/', such as '2024-01-01/2024-12-31', with " + "'..' for an open end ('2024-01-01/..'). Neither side can be a " + "duration." + ) + return sides[0], sides[1] def _all_blank(items: list[str | None]) -> bool: @@ -131,6 +149,7 @@ def _format_api_dates( *, name: str = "date input", single_value_hint: str = "an instant or a duration ('2020-01-01', 'P7D')", + open_start: str = _OPEN_BOUND, ) -> str | None: """ Formats date or datetime input(s) for use with an API. @@ -160,6 +179,12 @@ def _format_api_dates( forms (``get_ratings`` rejects durations) enforces that itself and passes a hint naming only what it accepts, so the remedy does not direct a caller to a value that is rejected. + open_start : str, optional + How the target API spells the open start of a range (``".."`` + by default). OGC API Features and STAC also allow an empty string, + ``"/2024-01-01"``, which an API whose firewall rejects ``"../"`` needs + instead; see :attr:`OgcDialect.open_start`. An open end is always + ``".."``. Returns ------- @@ -182,8 +207,11 @@ def _format_api_dates( or ``".."`` endpoint is rendered as ``".."`` to denote an open bound (e.g. ``"2024-01-01/.."``); the range is only None when *every* element is blank/NA/``".."``. - - Supports ISO 8601 durations such as "P7D" and "PT36H" and pre-formatted - intervals containing ``"/"``; both are passed through unchanged. + - Supports ISO 8601 durations such as "P7D" and "PT36H", which are passed + through unchanged. + - A single string containing ``"/"`` is an interval: it is split into the + two-value form, so ``"2024-01-01/.."`` and ``["2024-01-01", None]`` are + formatted alike. A duration is not accepted as either side. - Converts datetimes to UTC and formats as ISO 8601 with 'Z' suffix when `date` is False. Inputs with an explicit offset (``Z`` or ``+HH:MM``) are converted from that offset to UTC; naive inputs are interpreted in the @@ -193,6 +221,8 @@ def _format_api_dates( return None items = _coerce_to_list(datetime_input, name) + if len(items) == 1 and isinstance(items[0], str) and "/" in items[0]: + items = list(_split_interval(items[0], name=name)) if _all_blank(items): return None @@ -204,8 +234,11 @@ def _format_api_dates( "or two for a closed interval ('2020-01-01', '2020-12-31')." ) - # Pass through duration ("P7D", "PT36H") and pre-formatted interval ("a/b") - if len(items) == 1 and isinstance(items[0], str) and _is_passthrough(items[0]): + # Pass through a duration ("P7D", "PT36H") + if len(items) == 1 and isinstance(items[0], str) and _DURATION_RE.match(items[0]): return items[0] - return "/".join(_format_one(dt, date=date, name=name) for dt in items) + formatted = [_format_one(dt, date=date, name=name) for dt in items] + if len(formatted) == 2 and formatted[0] == _OPEN_BOUND: + formatted[0] = open_start + return "/".join(formatted) diff --git a/dataretrieval/ogc/policy.py b/dataretrieval/ogc/policy.py index c92c7da1..807fd48a 100644 --- a/dataretrieval/ogc/policy.py +++ b/dataretrieval/ogc/policy.py @@ -60,6 +60,12 @@ class OgcDialect: Columns to sort the combined result by, in priority order. Sorting is applied only when the first (primary) column is present; any later columns also present are added as secondary keys. + open_start : str + How the API spells the open start of a date range: ``".."`` + (``"../2024-01-01"``, the default) or ``""`` (``"/2024-01-01"``). + OGC API Features allows both, but deployments differ in which they + accept: the Water Data API rejects the empty start with HTTP 400, and + the NGWMN firewall rejects ``"../"`` with HTTP 403. """ cql2_services: frozenset[str] = field(default_factory=frozenset) @@ -67,6 +73,7 @@ class OgcDialect: time_cols: frozenset[str] = field(default_factory=frozenset) numerical_cols: frozenset[str] = field(default_factory=frozenset) sort_cols: tuple[str, ...] = field(default_factory=tuple) + open_start: str = ".." # Default dialect: a plain OGC API with no CQL2-only collections and no diff --git a/dataretrieval/ogc/requests.py b/dataretrieval/ogc/requests.py index 1427dc38..87e032ed 100644 --- a/dataretrieval/ogc/requests.py +++ b/dataretrieval/ogc/requests.py @@ -184,6 +184,7 @@ def _construct_api_requests( date=( collection in dialect.date_only_services and key != "last_modified" ), + open_start=dialect.open_start, ) params, post_params = _partition_request_params( kwargs, use_cql2=collection in dialect.cql2_services diff --git a/dataretrieval/waterdata/ratings.py b/dataretrieval/waterdata/ratings.py index f78ea2fc..6ceb2a53 100644 --- a/dataretrieval/waterdata/ratings.py +++ b/dataretrieval/waterdata/ratings.py @@ -179,7 +179,12 @@ def get_ratings( _validate_time_no_duration(time) time_str = ( _format_api_dates( - time, name="time", single_value_hint="an instant ('2020-01-01')" + time, + name="time", + single_value_hint="an instant ('2020-01-01')", + # The STAC firewall answers ``datetime=../2026-01-01`` with HTTP + # 403; STAC reads an empty start (``/2026-01-01``) as open. + open_start="", ) if time is not None else None diff --git a/tests/ngwmn_test.py b/tests/ngwmn_test.py index d73ae0f2..deeabb7a 100644 --- a/tests/ngwmn_test.py +++ b/tests/ngwmn_test.py @@ -460,6 +460,20 @@ def test_get_water_level_datetime_subsets(httpx_mock): assert qs["datetime"] == ["2022-01-01T00:00:00Z/2024-01-01T00:00:00Z"] +@pytest.mark.parametrize( + "datetime", [[None, "2016-01-01T00:00:00Z"], "../2016-01-01T00:00:00Z"] +) +def test_get_water_level_sends_an_open_start_empty(httpx_mock, datetime): + """The NGWMN firewall answers ``datetime=../2016-01-01`` with HTTP 403, so + an open start is sent as an empty string, which NGWMN reads as open.""" + _mock(httpx_mock, "waterLevelObs", _WATER_LEVELS) + + ngwmn.get_water_level(monitoring_location_id=_SITE, datetime=datetime) + + qs = _queries(httpx_mock, "waterLevelObs")[0] + assert qs["datetime"] == ["/2016-01-01T00:00:00Z"] + + def test_get_lithology(httpx_mock): _mock(httpx_mock, "lithologyObs", _LITHOLOGY) diff --git a/tests/waterdata_ratings_test.py b/tests/waterdata_ratings_test.py index f0c96967..aeedff0e 100644 --- a/tests/waterdata_ratings_test.py +++ b/tests/waterdata_ratings_test.py @@ -169,6 +169,23 @@ def test_get_ratings_download_and_parse_false_returns_features(httpx_mock): assert features[0]["id"] == "USGS-01104475.exsa.rdb" +@pytest.mark.parametrize( + "time", [[None, "2026-04-29T00:00:00Z"], "../2026-04-29T00:00:00Z"] +) +def test_get_ratings_sends_an_open_start_empty(httpx_mock, time): + """The STAC firewall answers ``datetime=../2026-04-29`` with HTTP 403, so an + open start is sent as an empty string, which STAC reads as open.""" + httpx_mock.add_response( + method="GET", url=STAC_SEARCH_RE, json=_stub_search_response() + ) + get_ratings( + monitoring_location_id="USGS-01104475", time=time, download_and_parse=False + ) + (request,) = httpx_mock.get_requests() + params = parse_qs(urlsplit(str(request.url)).query) + assert params["datetime"] == ["/2026-04-29T00:00:00Z"] + + def test_get_ratings_keeps_a_dotdot_open_bound_in_time(httpx_mock): """``time=[start, ".."]`` is the documented open-ended range. The ``..`` endpoint used to fail to parse, which discarded the whole range, so the diff --git a/tests/waterdata_test.py b/tests/waterdata_test.py index 541f7a24..966e0505 100644 --- a/tests/waterdata_test.py +++ b/tests/waterdata_test.py @@ -604,6 +604,15 @@ def test_get_args_materializes_numpy_and_series_numeric_params(): assert "bbox=-92.8%2C44.2%2C-88.9%2C46.0" in str(req.url) +def test_construct_api_requests_keeps_dotdot_for_an_open_start(): + """The Water Data API answers an empty start (``time=/2024-01-31``) with + HTTP 400, so its dialect keeps the default ``..``.""" + req = _construct_api_requests( + "daily", monitoring_location_id="USGS-05427718", time=[None, "2024-01-31"] + ) + assert "time=..%2F2024-01-31" in str(req.url) + + def test_construct_api_requests_two_element_date_list_becomes_interval(): """A two-element date list is interpreted as start/end of an OGC datetime interval (joined with '/'), NOT as two discrete dates. The OGC `datetime` @@ -801,7 +810,9 @@ def test_get_daily_keeps_a_dotdot_open_bound_in_time(httpx_mock): @pytest.mark.parametrize( - "time", ["2025-13-45", ["2025-01-01", "yesterday"]], ids=["single", "range"] + "time", + ["2025-13-45", ["2025-01-01", "yesterday"], "not-a-date/also-bad"], + ids=["single", "range", "interval_string"], ) def test_get_daily_rejects_an_unreadable_time_before_any_request(httpx_mock, time): """A bound that matches no date format used to drop the whole filter, so diff --git a/tests/waterdata_utils_test.py b/tests/waterdata_utils_test.py index 281744c0..6c22f040 100644 --- a/tests/waterdata_utils_test.py +++ b/tests/waterdata_utils_test.py @@ -948,7 +948,7 @@ def test_type_cols_warning_is_singular_for_one_value(): "fractional_seconds", "offset_to_utc", "iso8601_pair_to_interval", - "passthrough_interval", + "utc_interval_string", "passthrough_duration", "time_only_duration", "date_only", @@ -964,8 +964,8 @@ def test_type_cols_warning_is_singular_for_one_value(): def test_format_api_dates(value, date, expected): """``_format_api_dates`` normalizes ISO 8601 datetimes to UTC (dropping fractional seconds, converting offsets), joins a pair into an interval, - passes durations / intervals through unchanged, and renders a None - endpoint as ``..``.""" + passes durations through unchanged, and renders a None endpoint as + ``..``.""" assert _format_api_dates(value, date=date) == expected @@ -982,6 +982,118 @@ def test_format_api_dates_treats_an_all_blank_sequence_as_no_filter(): assert _format_api_dates(["..", ".."]) is None +@pytest.mark.parametrize( + "interval, pair, date", + [ + ("2024-01-01T10:00:00/..", ["2024-01-01T10:00:00", None], False), + ("../2024-01-01T10:00:00", [None, "2024-01-01T10:00:00"], False), + ("2018-02-12T19:20:50-04:00/..", ["2018-02-12T19:20:50-04:00", ".."], False), + ( + "2024-01-01 10:00:00/2024-01-02", + ["2024-01-01 10:00:00", "2024-01-02"], + False, + ), + ( + "2024-01-01T10:00:00Z/2024-02-01T00:00:00Z", + ["2024-01-01T10:00:00Z", "2024-02-01T00:00:00Z"], + True, + ), + ("2024-01-01T10:00:00Z/", ["2024-01-01T10:00:00Z", ""], False), + ], + ids=[ + "naive_start", + "naive_end", + "offset", + "space_separated", + "date_only_truncates", + "empty_end", + ], +) +def test_format_api_dates_formats_an_interval_string_like_the_pair( + interval, pair, date +): + """A ``"start/end"`` string and the two-value list are two spellings of the + same range, so they send the same value. The string used to be sent + unchanged: a naive time was not converted from local time to UTC, and a + date-only collection received times.""" + assert _format_api_dates(interval, date=date) == _format_api_dates(pair, date=date) + + +@pytest.mark.parametrize( + "interval, date, expected", + [ + ("2018-02-12T19:20:50-04:00/..", False, "2018-02-12T23:20:50Z/.."), + ("2024-01-01T10:00:00Z/2024-02-01T00:00:00Z", True, "2024-01-01/2024-02-01"), + ], + ids=["offset_to_utc", "date_only"], +) +def test_format_api_dates_formats_each_side_of_an_interval_string( + interval, date, expected +): + """Each side is formatted on its own, as a list element is.""" + assert _format_api_dates(interval, date=date) == expected + + +def test_format_api_dates_treats_an_all_open_interval_string_as_no_filter(): + """``"../.."`` is the string spelling of ``[None, None]``.""" + assert _format_api_dates("../..") is None + + +@pytest.mark.parametrize( + "value", + [ + [None, "2024-01-01T00:00:00Z"], + ["..", "2024-01-01T00:00:00Z"], + "../2024-01-01T00:00:00Z", + "/2024-01-01T00:00:00Z", + ], + ids=["none", "dotdot", "dotdot_string", "empty_string"], +) +def test_format_api_dates_spells_an_open_start_as_the_api_does(value): + """Every spelling of an open start becomes ``..`` by default and the + caller's ``open_start`` otherwise: NGWMN and STAC reject ``"../"`` (HTTP + 403), and the Water Data API rejects an empty start (HTTP 400).""" + assert _format_api_dates(value) == "../2024-01-01T00:00:00Z" + assert _format_api_dates(value, open_start="") == "/2024-01-01T00:00:00Z" + + +def test_format_api_dates_open_start_leaves_an_open_end_and_no_filter_alone(): + """``open_start`` changes only the start: an open end is ``..`` on every API, + and a range with both ends open is still no filter.""" + open_end = _format_api_dates(["2024-01-01T00:00:00Z", None], open_start="") + assert open_end == "2024-01-01T00:00:00Z/.." + assert _format_api_dates([None, None], open_start="") is None + assert _format_api_dates("../..", open_start="") is None + + +@pytest.mark.parametrize( + "interval, message", + [ + ("not-a-date/also-bad", "time could not be read as a date or datetime"), + ("2024-01-01/tomorrow", "time could not be read as a date or datetime"), + ("2024-01-01/2024-02-01/2024-03-01", "time is not a valid interval"), + # The Water Data and NGWMN services answer a duration side with HTTP 400. + ("2024-01-01T10:00:00Z/PT36H", "time is not a valid interval"), + ("P7D/2024-01-08", "time is not a valid interval"), + ("P7D/..", "time is not a valid interval"), + ], + ids=[ + "both_sides", + "one_side", + "three_sides", + "start_duration", + "duration_end", + "duration_open_end", + ], +) +def test_format_api_dates_rejects_an_unreadable_interval_string(interval, message): + """An interval string used to be sent unchanged, so ``time="garbage/.."`` + reached the service. It is now rejected like an unreadable list element, + naming the caller's argument.""" + with pytest.raises(ValueError, match=f"^{message}"): + _format_api_dates(interval, name="time") + + @pytest.mark.parametrize( "value", ["2024-13-45", "Jan 1 2024", "Apr", ["2024-01-01", "garbage"], ["garbage", None]],