diff --git a/.importlinter b/.importlinter index 7dc23e52..61fa1368 100644 --- a/.importlinter +++ b/.importlinter @@ -27,6 +27,9 @@ layers = wateruse ngwmn | nldi | nwdc | nwis | streamstats | waterdata | wqp ogc +; The ISO 8601 interval grammar is shared by OGC and STAC, which is not OGC, +; so it sits below ``ogc`` rather than inside it (ADR 0014). + _intervals utils _querying transport @@ -42,7 +45,7 @@ layers = ; Pure dependency-free mechanisms sit together at the floor: none imports ; first-party code, so every layer can use them without reaching sideways or ; creating artificial dependencies among them. - _ambient | _csv | _deprecation | _validation + _ambient | _csv | _dates | _deprecation | _validation ; Every top-level module must be placed in the stack deliberately. A new ; top-level module fails this contract until someone decides where it sits. exhaustive = True diff --git a/CONTEXT.md b/CONTEXT.md index 2339dfc3..0367f236 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -312,9 +312,9 @@ entry (ADR 0013). OGC internals, the Water Data wrappers, and all eleven typed getters now say `collection`; `waterdata.get_cql` takes `collection`; and the type alias is `WATERDATA_COLLECTIONS`. `service=` on `get_cql` and the `WATERDATA_SERVICES` - alias remain — a deprecated keyword and a permanent alias respectively. One - `OgcDialect` field also still says it: `date_only_services` is keyed by - collection. + alias remain — a deprecated keyword and a permanent alias respectively. The + dialect field keyed by collection says so too: + `OgcDialect.date_only_collections`. `service` still means the external system in `transport` and `progress`, where it labels a progress line. That usage is correct. diff --git a/NEWS.md b/NEWS.md index 1dda2416..eaeafe6b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,5 @@ +**10/12/2026:** **Behavior change:** a lone ISO 8601 duration, such as `datetime="P7D"`, passed to `ngwmn.get_water_level()` or another `ngwmn` getter now raises `ValueError` before any request is sent; the NGWMN service rejects one with HTTP 400. `waterdata.get_ratings()` already rejected a duration, and its message now matches the other getters'. The `waterdata` OGC getters still accept durations. Date arguments are now read in `dataretrieval._dates` and written in `dataretrieval._intervals` (ADR 0014, proposed); the values sent are otherwise unchanged. + **10/12/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 in `waterdata.get_ratings()`. **Bug fix:** the string used to be sent unchanged, so the two forms could select different data: an offset was not converted to `Z`, and a date-only collection such as `daily` received times. A string now raises `ValueError` before any request is sent if a side is not a date (`time="2024-01-01/tomorrow"`), if it contains more than one `/`, or if either side is a duration (`"2024-01-01/P7D"`), which the services rejected 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:** in `ngwmn.get_water_level()` and `waterdata.get_ratings()`, a range with an open start, such as `[None, "2016-01-01"]` or `"../2016-01-01"`, failed with HTTP 403 because those services' firewalls reject `../`. Both getters now send an empty start (`"/2016-01-01"`), which both services treat as open. The other Water Data getters still send `..`, because that API rejects an empty start. Fractional seconds, such as `2024-01-01T10:00:00.999Z`, are kept instead of truncated, and a minute-precision time (`2024-01-01T10:00Z`) and spaces around the `/` are accepted. **Behavior change:** a naive date or time is now UTC, as in pandas (`pd.to_datetime(..., utc=True)`) and `get_nearest_continuous()`, instead of the machine's local time, so `time=["2024-03-01", "2024-03-02"]` sends `2024-03-01T00:00:00Z/2024-03-02T00:00:00Z` on every machine. An aware value is converted to UTC, including in a date-only collection such as `daily`. For a local time, include the offset: `"2024-03-01T00:00:00-06:00"`. **10/12/2026:** Date arguments of the `waterdata` and `ngwmn` OGC getters (`time`, `begin`, `end`, `datetime`, `last_modified`) and of `waterdata.get_ratings()` accept `datetime.date`, `datetime.datetime`, and `pandas.Timestamp` values, alone or as either end of a range, such as `time=[df.index.min(), None]`. Each is read like the equivalent string: a `date` is midnight, and `NaT` is an open bound. **Bug fix:** these values used to raise `AttributeError` or `TypeError` without naming the argument. Any other type, such as a number, now raises `ValueError` naming the argument and the accepted types. diff --git a/dataretrieval/_dates.py b/dataretrieval/_dates.py new file mode 100644 index 00000000..6f44d632 --- /dev/null +++ b/dataretrieval/_dates.py @@ -0,0 +1,263 @@ +"""What a caller meant by a date argument (ADR 0014). + +A dependency-free leaf. A date argument is read here into a :class:`DateRange`, +and the protocol code that sends the request writes that value in its own wire +format. Reading -- which types and string formats are accepted, what is an open +bound, and that a naive value is UTC -- is package policy, so it lives here and +names no service. +""" + +from __future__ import annotations + +import re +from collections.abc import Iterable, Mapping, Sequence +from dataclasses import dataclass +from datetime import date as _date +from datetime import datetime, timezone + +import pandas as pd + +_DATETIME_FORMATS = ( + "%Y-%m-%dT%H:%M:%S.%f%z", + "%Y-%m-%dT%H:%M:%S%z", + "%Y-%m-%dT%H:%M:%S.%f", + "%Y-%m-%dT%H:%M:%S", + "%Y-%m-%dT%H:%M%z", + "%Y-%m-%dT%H:%M", + "%Y-%m-%d %H:%M:%S.%f", + "%Y-%m-%d %H:%M:%S", + "%Y-%m-%d", +) + + +# Anchored to ``[Pp]\d`` so a normal word containing ``p`` (e.g. ``"Apr"``) +# doesn't get mis-classified as an ISO 8601 duration; the optional ``T`` +# admits time-only forms like ``PT36H``. +_DURATION_RE = re.compile(r"^[Pp]T?\d") + + +# The ISO 8601 open interval bound, as in ``"2024-01-01/.."``. +OPEN_BOUND = ".." + + +# One element of a date argument: an ISO 8601 string, or a ``date``, +# ``datetime`` or ``pandas.Timestamp`` (both subclasses of ``datetime.date``), +# with ``None`` (or ``NaN`` / ``NaT``) for an open bound. +DateLike = str | _date | None + + +@dataclass(frozen=True) +class DateRange: + """One date argument as the caller meant it. + + Exactly one shape is set: an instant (``instant=True``, ``start`` only), a + range of two bounds where ``None`` is an open bound (``start``, ``end``), + or an ISO 8601 duration (``duration``, such as ``"P7D"``). Every bound is + in UTC: a naive value is read as UTC, as in pandas, and an aware one is + converted. + """ + + start: datetime | None = None + end: datetime | None = None + instant: bool = False + duration: str | None = None + + +def _parse_datetime(value: str) -> datetime | None: + """Parse a single datetime string against the supported formats. + + Returns a ``datetime`` (tz-aware iff the input included a UTC offset), + or ``None`` if no format matched. + """ + # ``datetime.strptime`` accepts a numeric offset like ``+00:00`` but not + # the ``Z`` shorthand, so normalize trailing ``Z`` first. + candidate = value[:-1] + "+00:00" if value.endswith("Z") else value + for fmt in _DATETIME_FORMATS: + try: + # DTZ007: naive is the documented outcome for a naive input -- + # ``_DATETIME_FORMATS`` carries both the ``%z`` and the bare forms. + return datetime.strptime(candidate, fmt) # noqa: DTZ007 + except ValueError: + continue + return None + + +def _is_blank(dt: DateLike) -> bool: + """True for a None, NaN, empty-string, or ``..`` element. + + Each is an open bound, so ``["2024-01-01", ".."]`` means the same range as + ``["2024-01-01", None]`` and the string ``"2024-01-01/.."``. + """ + return dt is None or bool(pd.isna(dt)) or dt in ("", OPEN_BOUND) + + +def _to_datetime(dt: str | _date, *, name: str) -> datetime: + """Read one non-blank element as a ``datetime`` (naive iff it has no zone). + + A ``date`` is midnight of that day, like the string ``"2024-01-01"``. + Raises ``ValueError`` naming *name* for an unreadable string or a value of + another type. + """ + if isinstance(dt, pd.Timestamp): + # ``Timestamp.astimezone`` is ``tz_convert`` and requires a zone, so + # return a plain ``datetime``. + converted: datetime = dt.to_pydatetime(warn=False) + return converted + if isinstance(dt, datetime): + return dt + if isinstance(dt, _date): + return datetime(dt.year, dt.month, dt.day) # noqa: DTZ001 + parsed = _parse_datetime(dt) if isinstance(dt, str) else None + if parsed is None: + raise ValueError( + f"{name} could not be read as a date or datetime: {dt!r}. " + "Pass an ISO 8601 date or datetime such as '2024-01-01' or " + "'2024-01-01T12:00:00Z' (a datetime.date, datetime.datetime or " + "pandas.Timestamp also works), and None for an open end of a range " + "(['2024-01-01', None])." + ) + return parsed + + +def _read_bound(dt: DateLike, *, name: str) -> datetime | None: + """One element as a UTC ``datetime``, or None for an open bound.""" + if dt is None or _is_blank(dt): + return None + parsed = _to_datetime(dt, name=name) + # As in pandas, a naive value is UTC and an aware one is converted to UTC, + # so the same call sends the same range on every machine. + if parsed.tzinfo is None: + return parsed.replace(tzinfo=timezone.utc) + return parsed.astimezone(timezone.utc) + + +def _coerce_to_list( + datetime_input: str | _date | Sequence[DateLike], + name: str = "date input", +) -> list[DateLike]: + """Normalize datetime input to a list, raising on invalid shapes.""" + if isinstance(datetime_input, (str, bytes)) or not isinstance( + datetime_input, Iterable + ): + # A lone value (a ``date`` is not iterable; ``bytes`` would iterate as + # integers); a non-date is rejected per element. + return [datetime_input] + if isinstance(datetime_input, Mapping): + raise TypeError( + f"{name} must be a date, a string, or a sequence of them, " + f"not {type(datetime_input).__name__}." + ) + return list(datetime_input) + + +def _split_interval(interval: str, *, name: str) -> tuple[str, str]: + """Split a ``"start/end"`` string into its two sides. + + Each side is then read like a list element, so ``"2024-01-01T10:00:00/.."`` + means 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 reject ``"2024-01-01/P7D"`` and ``"P7D/2024-01-08"`` with + HTTP 400. + """ + sides = [side.strip() for side in 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 read_date_range( + value: str | _date | Sequence[DateLike] | None, + *, + name: str, + durations: bool, +) -> DateRange | None: + """Read a date argument as the caller meant it. + + Parameters + ---------- + value : str, date, sequence of them, or None + A single date/datetime, a ``"start/end"`` interval string, an ISO 8601 + duration (``"P7D"``, ``"PT36H"``), or a list of one or two elements. + Each element may be a ``datetime.date``, ``datetime.datetime`` or + ``pandas.Timestamp``, or a string in ISO 8601 (with or without ``Z`` or + a numeric offset) or ``"%Y-%m-%d %H:%M:%S"`` form. ``None``, ``NaN``, + ``NaT``, ``""`` and ``".."`` are open bounds. + name : str + The caller's own spelling of this argument, used as the subject of + every message raised here, so a caller correcting the error edits an + argument their getter accepts. + durations : bool + Whether the API accepts a lone ISO 8601 duration; see + ``DateDialect.durations``. When it does not, a duration raises, and no + message offers one. + + Returns + ------- + DateRange or None + ``None`` when *value* is ``None`` or every element is an open bound, + meaning no filter. + + Raises + ------ + ValueError + For more than two elements, an unreadable interval string, a duration + where *durations* is false, or an element that is not blank and matches + no supported format or type. + TypeError + For a mapping. + """ + if value is None: + return None + + items = _coerce_to_list(value, name) + if len(items) == 1 and isinstance(items[0], str) and "/" in items[0]: + items = list(_split_interval(items[0], name=name)) + + if all(_is_blank(dt) for dt in items): + return None + + if len(items) > 2: + raise _too_many_values(items, name=name, durations=durations) + if len(items) == 1: + return _read_single(items[0], name=name, durations=durations) + return DateRange( + start=_read_bound(items[0], name=name), end=_read_bound(items[1], name=name) + ) + + +def _too_many_values( + items: list[DateLike], *, name: str, durations: bool +) -> ValueError: + """The error for more than two elements; it offers a duration only where + one is accepted.""" + single = ( + "an instant or a duration ('2020-01-01', 'P7D')" + if durations + else "an instant ('2020-01-01')" + ) + return ValueError( + f"{name} takes at most 2 values, got {len(items)}: {items!r}. " + f"Pass one value for {single}, " + "or two for a closed interval ('2020-01-01', '2020-12-31')." + ) + + +def _read_single(item: DateLike, *, name: str, durations: bool) -> DateRange: + """One non-blank element: a duration, or an instant.""" + if isinstance(item, str) and _DURATION_RE.match(item): + if not durations: + raise ValueError( + f"{name} cannot be an ISO 8601 duration for this API: " + f"{item!r}. Pass a start and an end instead, such as " + "['2024-01-01', '2024-01-08'], with None for an open end " + "(['2024-01-01', None])." + ) + return DateRange(duration=item) + return DateRange(start=_read_bound(item, name=name), instant=True) diff --git a/dataretrieval/_intervals.py b/dataretrieval/_intervals.py new file mode 100644 index 00000000..943d4bc6 --- /dev/null +++ b/dataretrieval/_intervals.py @@ -0,0 +1,76 @@ +"""The ISO 8601 interval grammar OGC API Features and STAC share (ADR 0014). + +A date argument is read into a :class:`~dataretrieval._dates.DateRange`; this +module writes that value as the ``datetime`` parameter of both standards -- an +instant, a ``start/end`` interval with ``..`` open bounds, or a duration -- +adjusted by the API's :class:`DateDialect`. It names no service. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime + +from dataretrieval._dates import OPEN_BOUND, DateRange + + +@dataclass(frozen=True) +class DateDialect: + """How one API writes dates differently from the interval standard. + + Each default is the standard, so ``DateDialect()`` describes an API that + follows it. Where a field is set, the declaration records the observed + server behavior that requires it. + + Attributes + ---------- + open_start : str + The string written for the open start of a range: ``".."`` + (``"../2024-01-01"``, the default) or ``""`` (``"/2024-01-01"``). Both + standards allow either, but each service accepts only one. + durations : bool + Whether a lone ISO 8601 duration (``"P7D"``, the last seven days) is + accepted. Neither standard defines one, so the default is ``False`` + and a duration raises before any request is sent. + """ + + open_start: str = OPEN_BOUND + durations: bool = False + + +def _write_bound(bound: datetime | None, *, date_only: bool) -> str: + """One UTC bound: ``YYYY-MM-DD``, ``YYYY-MM-DDTHH:MM:SS[.ffffff]Z``, or ``..``.""" + if bound is None: + return OPEN_BOUND + if date_only: + return bound.strftime("%Y-%m-%d") + # Keep fractional seconds: truncating them changes a closed bound. + fraction = f".{bound.microsecond:06d}".rstrip("0") if bound.microsecond else "" + return f"{bound.strftime('%Y-%m-%dT%H:%M:%S')}{fraction}Z" + + +def write_interval(value: DateRange, dialect: DateDialect, *, date_only: bool) -> str: + """Write a date range as the API's date parameter expects it. + + Parameters + ---------- + value : DateRange + What the caller meant, as read by + :func:`~dataretrieval._dates.read_date_range`, which refuses a duration + the dialect does not accept. + dialect : DateDialect + The API's date dialect; supplies ``open_start``. + date_only : bool + Write each bound as its UTC date, ``YYYY-MM-DD``, rather than as a UTC + datetime; the caller's protocol decides which values are calendar days. + """ + if value.duration is not None: + return value.duration + if value.instant: + return _write_bound(value.start, date_only=date_only) + start = ( + dialect.open_start + if value.start is None + else _write_bound(value.start, date_only=date_only) + ) + return f"{start}/{_write_bound(value.end, date_only=date_only)}" diff --git a/dataretrieval/ngwmn.py b/dataretrieval/ngwmn.py index eb0c3dbf..52935890 100644 --- a/dataretrieval/ngwmn.py +++ b/dataretrieval/ngwmn.py @@ -24,6 +24,7 @@ import pandas as pd from dataretrieval import configuration as _configuration +from dataretrieval._intervals import DateDialect from dataretrieval.codes.states import apply_state from dataretrieval.configuration import ( BaseConfiguration, @@ -86,7 +87,8 @@ # sort vocabulary: water-level observations are timestamped by ``sample_time`` # (not the Water Data ``time``) and report depths/levels in feet. Its firewall # rejects ``datetime=../2016-01-01`` with HTTP 403, so an open start is sent -# empty (``/2016-01-01``), which NGWMN treats as open. +# empty (``/2016-01-01``), which NGWMN treats as open. It rejects a lone +# duration (``datetime=P7D``) with HTTP 400, the standard's default. NGWMN_DIALECT = OgcDialect( time_cols=frozenset({"sample_time"}), numerical_cols=frozenset( @@ -97,7 +99,7 @@ } ), sort_cols=("sample_time", "monitoring_location_id"), - open_start="", + dates=DateDialect(open_start=""), ) @@ -110,7 +112,7 @@ def _get(service: str, local_vars: dict[str, Any]) -> tuple[pd.DataFrame, BaseMe queryable = _STATE_QUERYABLE.get(service) if queryable is not None: apply_state(local_vars, to=queryable["to"], into=queryable["into"]) - args = prepare_request_args(local_vars) + args = prepare_request_args(local_vars, dialect=NGWMN_DIALECT) return get_ogc_data( args, service, diff --git a/dataretrieval/ogc/dates.py b/dataretrieval/ogc/dates.py deleted file mode 100644 index a96beddf..00000000 --- a/dataretrieval/ogc/dates.py +++ /dev/null @@ -1,280 +0,0 @@ -"""Date and datetime marshalling for OGC time parameters. - -Pure helpers that render the OGC getters' time-shaped arguments — single -instants, two-element ``[start, end]`` ranges, ISO-8601 durations, and open -``..`` bounds — into the wire form the API expects. No I/O, no engine state. -""" - -from __future__ import annotations - -import re -from collections.abc import Iterable, Mapping, Sequence -from datetime import date as _date -from datetime import datetime -from zoneinfo import ZoneInfo - -import pandas as pd - -_DATETIME_FORMATS = ( - "%Y-%m-%dT%H:%M:%S.%f%z", - "%Y-%m-%dT%H:%M:%S%z", - "%Y-%m-%dT%H:%M:%S.%f", - "%Y-%m-%dT%H:%M:%S", - "%Y-%m-%dT%H:%M%z", - "%Y-%m-%dT%H:%M", - "%Y-%m-%d %H:%M:%S.%f", - "%Y-%m-%d %H:%M:%S", - "%Y-%m-%d", -) - - -# Anchored to ``[Pp]\d`` so a normal word containing ``p`` (e.g. ``"Apr"``) -# doesn't get mis-classified as an ISO 8601 duration; the optional ``T`` -# admits time-only forms like ``PT36H``. -_DURATION_RE = re.compile(r"^[Pp]T?\d") - - -# OGC API parameters that take a date/datetime value (single string, -# two-element range, or interval/duration string) rather than a multi-value -# string list. Used by ``_construct_api_requests`` to keep them out of the -# POST/CQL2 multi-value path and to route them through ``_format_api_dates``, -# and by the default ``_get_args`` no-normalize set to bypass string-iterable -# normalization. -_DATE_RANGE_PARAMS = frozenset( - {"datetime", "last_modified", "begin", "begin_utc", "end", "end_utc", "time"} -) - - -def _parse_datetime(value: str) -> datetime | None: - """Parse a single datetime string against the supported formats. - - Returns a ``datetime`` (tz-aware iff the input included a UTC offset), - or ``None`` if no format matched. - """ - # ``datetime.strptime`` accepts a numeric offset like ``+00:00`` but not - # the ``Z`` shorthand, so normalize trailing ``Z`` first. - candidate = value[:-1] + "+00:00" if value.endswith("Z") else value - for fmt in _DATETIME_FORMATS: - try: - # DTZ007: naive is the documented outcome for a naive input -- - # ``_DATETIME_FORMATS`` carries both the ``%z`` and the bare forms. - return datetime.strptime(candidate, fmt) # noqa: DTZ007 - except ValueError: - continue - return None - - -# The OGC API spelling of an open interval bound, as in ``"2024-01-01/.."``. -_OPEN_BOUND = ".." - - -# One element of a date argument: an ISO 8601 string, or a ``date``, -# ``datetime`` or ``pandas.Timestamp`` (both subclasses of ``datetime.date``), -# with ``None`` (or ``NaN`` / ``NaT``) for an open bound. -_DateLike = str | _date | None - - -def _is_blank(dt: _DateLike) -> bool: - """True for a None, NaN, empty-string, or ``..`` element. - - Each is a spelling of an open bound, so ``["2024-01-01", ".."]`` means the - same range as ``["2024-01-01", None]`` and the string ``"2024-01-01/.."``. - """ - return dt is None or bool(pd.isna(dt)) or dt in ("", _OPEN_BOUND) - - -def _to_datetime(dt: str | _date, *, name: str) -> datetime: - """Read one non-blank element as a ``datetime`` (naive iff it has no zone). - - A ``date`` is midnight of that day, like the string ``"2024-01-01"``. - Raises ``ValueError`` naming *name* for an unreadable string or a value of - another type. - """ - if isinstance(dt, pd.Timestamp): - # ``Timestamp.astimezone`` is ``tz_convert`` and requires a zone, so - # hand ``_format_one`` a plain ``datetime``. - converted: datetime = dt.to_pydatetime(warn=False) - return converted - if isinstance(dt, datetime): - return dt - if isinstance(dt, _date): - return datetime(dt.year, dt.month, dt.day) # noqa: DTZ001 - parsed = _parse_datetime(dt) if isinstance(dt, str) else None - if parsed is None: - raise ValueError( - f"{name} could not be read as a date or datetime: {dt!r}. " - "Pass an ISO 8601 date or datetime such as '2024-01-01' or " - "'2024-01-01T12:00:00Z' (a datetime.date, datetime.datetime or " - "pandas.Timestamp also works), and None for an open end of a range " - "(['2024-01-01', None])." - ) - return parsed - - -def _format_one(dt: _DateLike, *, date: bool, name: str) -> str: - """Format a single datetime element for inclusion in the API time arg. - - Raises ``ValueError`` naming *name* when the element is not blank and - cannot be read as a date or datetime. - """ - if dt is None or _is_blank(dt): - return _OPEN_BOUND - parsed = _to_datetime(dt, name=name) - # As in pandas, a naive value is UTC and an aware one is converted to UTC, - # so the same call sends the same range on every machine. - utc = parsed if parsed.tzinfo is None else parsed.astimezone(ZoneInfo("UTC")) - if date: - return utc.strftime("%Y-%m-%d") - # Keep fractional seconds: truncating them changes a closed bound. - fraction = f".{utc.microsecond:06d}".rstrip("0") if utc.microsecond else "" - return f"{utc.strftime('%Y-%m-%dT%H:%M:%S')}{fraction}Z" - - -def _coerce_to_list( - datetime_input: str | _date | Sequence[_DateLike], - name: str = "date input", -) -> list[_DateLike]: - """Normalize datetime input to a list, raising on invalid shapes.""" - if isinstance(datetime_input, (str, bytes)) or not isinstance( - datetime_input, Iterable - ): - # A lone value (a ``date`` is not iterable; ``bytes`` would iterate as - # integers); a non-date is rejected per element. - return [datetime_input] - if isinstance(datetime_input, Mapping): - raise TypeError( - f"{name} must be a date, a string, or a sequence of them, " - f"not {type(datetime_input).__name__}." - ) - return list(datetime_input) - - -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 reject ``"2024-01-01/P7D"`` and ``"P7D/2024-01-08"`` with - HTTP 400. - """ - sides = [side.strip() for side in 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[_DateLike]) -> bool: - """True when every element is None, NaN, the empty string, or ``..``.""" - return all(_is_blank(dt) for dt in items) - - -def _format_api_dates( - datetime_input: str | _date | Sequence[_DateLike] | None, - date: bool = False, - *, - 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. - - Handles single values or ranges, converting to ISO 8601 or date-only - formats as needed. - - Parameters - ---------- - datetime_input : Union[str, date, List[Optional[Union[str, date]]], None] - A single date/datetime or a list of one or two of them. Each may be a - ``datetime.date``, ``datetime.datetime`` or ``pandas.Timestamp`` (a - naive one is read in the local time zone, as a naive string is), or a - string. Strings accept formats like "%Y-%m-%d %H:%M:%S", ISO 8601 (with or - without ``Z``/numeric offset), or relative periods (e.g., "P7D" / - "PT36H"). Range endpoints may be ``None``/``NaN``/empty or ``".."`` - to denote a half-bounded range. - date : bool, optional - If True, uses only the date portion ("YYYY-MM-DD"). If False (default), - returns full datetime in UTC ISO 8601 format ("YYYY-MM-DDTHH:MM:SSZ"). - name : str, optional - The caller's own spelling of this argument, used as the subject of - every message raised here. Defaults to a generic "date input"; pass - the real parameter name (``"time"``, ``"last_modified"``) so a caller - correcting the error edits an argument their getter accepts. - single_value_hint : str, optional - How the "too many values" message describes an acceptable single - value. Wording only -- a getter that rejects some of the default's - 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 - The string written for an open range start (``".."`` by default). - OGC API Features and STAC also allow an empty string - (``"/2024-01-01"``), which an API whose firewall rejects ``"../"`` - requires; see :attr:`OgcDialect.open_start`. An open end is always - ``".."``. - - Returns - ------- - Union[str, None] - - If input is a single value, returns the formatted date/datetime string. - - If input is a list of two values, returns a date/datetime range string - separated by "/" (e.g., "YYYY-MM-DD/YYYY-MM-DD" or - "YYYY-MM-DDTHH:MM:SSZ/YYYY-MM-DDTHH:MM:SSZ"). - - Returns None if input is None, empty, or every element is blank. - - Raises - ------ - ValueError - If `datetime_input` contains more than two values, or an element that - is not blank matches no supported format or is of another type. - - Notes - ----- - - A single blank/NA value returns None. In a two-value range, a blank/NA - or ``".."`` endpoint is an open bound. An open end is written ``".."`` - (``"2024-01-01/.."``) and an open start as ``open_start``. The range is - None only when *every* element is blank/NA/``".."``. - - 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. As in pandas, an input with an offset (``Z`` or - ``+HH:MM``) is converted to UTC and a naive one is UTC; a date-only - collection gets the UTC date. - """ - if datetime_input is None: - 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 - - if len(items) > 2: - raise ValueError( - f"{name} takes at most 2 values, got {len(items)}: {items!r}. " - f"Pass one value for {single_value_hint}, " - "or two for a closed interval ('2020-01-01', '2020-12-31')." - ) - - # Pass through a duration ("P7D", "PT36H") - if len(items) == 1 and isinstance(items[0], str) and _DURATION_RE.match(items[0]): - return items[0] - - 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/engine.py b/dataretrieval/ogc/engine.py index b4aa891f..28acd4d1 100644 --- a/dataretrieval/ogc/engine.py +++ b/dataretrieval/ogc/engine.py @@ -5,8 +5,7 @@ pagination and sync dispatch are in :mod:`dataretrieval.transport`; request construction is in :mod:`~dataretrieval.ogc.requests`. The surrounding concerns are in sibling modules this one composes, each with its own reason -to change: :mod:`~dataretrieval.ogc.dates` (time-parameter marshalling), -:mod:`~dataretrieval.ogc.errors` (HTTP error mapping), and +to change: :mod:`~dataretrieval.ogc.errors` (HTTP error mapping) and :mod:`~dataretrieval.ogc.shaping` (GeoJSON features to DataFrame and result finalization). It is deliberately free of any Water-Data-specific constants so a sibling package (e.g. NGWMN) can use it without importing @@ -47,6 +46,7 @@ from dataretrieval.ogc.requests import ( _construct_api_requests, _construct_cql_request, + _read_raw_dates, _switch_arg_id, _switch_properties_id, ) @@ -308,6 +308,7 @@ def get_ogc_data( # The APIs answer a blank filter with HTTP 400. del args["filter"] args.pop("filter_lang", None) + args = _read_raw_dates(args, dialect) # Choose one semantic frame shape for the whole request. Every page and # the all-empty finalizer receive these same values, so frame type never diff --git a/dataretrieval/ogc/planning.py b/dataretrieval/ogc/planning.py index 0f7ae045..193502d4 100644 --- a/dataretrieval/ogc/planning.py +++ b/dataretrieval/ogc/planning.py @@ -37,20 +37,13 @@ # Any list-shaped kwarg with >1 element is chunked (comma-joined per sub-list in the # URL); ~90 OGC params qualify, so the few exceptions are denylisted rather than # maintaining a growing allowlist. Excluded because: ``properties`` defines the column -# schema; ``bbox`` is a fixed coord tuple; date/time params are intervals, not -# enumerable sets; ``filter`` is handled as its own OR-axis in ``_extract_axes``; and -# ``limit`` / ``skip_geometry`` / ``filter_lang`` are scalar by contract. +# schema; ``bbox`` is a fixed coord tuple; ``filter`` is handled as its own OR-axis in +# ``_extract_axes``; and ``limit`` / ``skip_geometry`` / ``filter_lang`` are scalar by +# contract. A date parameter needs no entry: it arrives as a ``DateRange``, not a list. _NEVER_CHUNK = frozenset( { "properties", "bbox", - "datetime", - "last_modified", - "begin", - "begin_utc", - "end", - "end_utc", - "time", "filter", "filter_lang", "limit", diff --git a/dataretrieval/ogc/policy.py b/dataretrieval/ogc/policy.py index 5094a815..a656f9b5 100644 --- a/dataretrieval/ogc/policy.py +++ b/dataretrieval/ogc/policy.py @@ -2,8 +2,8 @@ This module is the one definition of the :class:`OgcDialect` type (per-API quirks the generic request builder needs) and OGC control validation. -It depends only on the stdlib, so any OGC submodule can import it without -creating cycles. +It depends only on the stdlib and the interval grammar's dialect type, so any +OGC submodule can import it without creating cycles. It names no endpoint: which API an OGC call targets is the *adapter's* policy, supplied per call as ``base_url``. A default here would @@ -17,6 +17,8 @@ import numbers from dataclasses import dataclass, field +from dataretrieval._intervals import DateDialect + def _require_positive_int( value: int, name: str, *, examples: str | None = None @@ -46,11 +48,15 @@ class OgcDialect: Whether a filter with more than one value is sent as a CQL2 ``in`` predicate, rather than as a comma-joined GET value (``site=A,B``, the default) that the API would read as one value. - date_only_services : frozenset[str] - Collections whose time arguments are rendered date-only - (``YYYY-MM-DD``) rather than as a full UTC datetime. The - ``last_modified`` parameter is always rendered as a full datetime - regardless of this set. + date_params : frozenset[str] + The parameters that carry a date range. The standard has one, + ``datetime``; an API that filters other fields by date lists them all. + date_only_collections : frozenset[str] + Collections whose dates are written as ``YYYY-MM-DD`` rather than as a + UTC datetime, because their values are calendar days. + timed_params : frozenset[str] + Date parameters written as a UTC datetime even in a date-only + collection, because they filter a timestamp. time_cols : frozenset[str] Result columns to coerce to datetime when ``convert_type`` is set. Empty by default, so the generic engine holds no API-specific column list; each API @@ -61,20 +67,19 @@ 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 - The string written for the open start of a date range: ``".."`` - (``"../2024-01-01"``, the default) or ``""`` (``"/2024-01-01"``). - OGC API Features allows both, but each service accepts only one: the - Water Data API rejects an empty start with HTTP 400, and the NGWMN - firewall rejects ``"../"`` with HTTP 403. + dates : DateDialect + How the API writes date ranges: the interval grammar it shares with + STAC. The default follows the standard. """ lists_as_cql2: bool = False - date_only_services: frozenset[str] = field(default_factory=frozenset) + date_params: frozenset[str] = frozenset({"datetime"}) + date_only_collections: frozenset[str] = field(default_factory=frozenset) + timed_params: frozenset[str] = field(default_factory=frozenset) 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 = ".." + dates: DateDialect = field(default_factory=DateDialect) # Default dialect: a plain OGC API, with lists comma-joined in the URL and every diff --git a/dataretrieval/ogc/requests.py b/dataretrieval/ogc/requests.py index 8f082daa..a8e8ec26 100644 --- a/dataretrieval/ogc/requests.py +++ b/dataretrieval/ogc/requests.py @@ -16,7 +16,8 @@ import httpx -from dataretrieval.ogc.dates import _DATE_RANGE_PARAMS, _format_api_dates +from dataretrieval._dates import DateRange, read_date_range +from dataretrieval._intervals import write_interval from dataretrieval.ogc.filters import _cql_in_clause from dataretrieval.ogc.policy import DEFAULT_DIALECT, OgcDialect from dataretrieval.transport.http import default_headers as _default_headers @@ -183,6 +184,19 @@ def _cql2_post_request( ) +def _write_date_param( + value: Any, key: str, collection: str, dialect: OgcDialect +) -> str | None: + """Write one date argument as the API expects, or None for no filter.""" + read = _read_date(key, value, dialect) + if read is None: + return None + date_only = ( + collection in dialect.date_only_collections and key not in dialect.timed_params + ) + return write_interval(read, dialect.dates, date_only=date_only) + + def _construct_api_requests( collection: str, properties: list[str] | None = None, @@ -203,16 +217,8 @@ def _construct_api_requests( service_url = _items_url(collection, base_url) if dialect is None: dialect = DEFAULT_DIALECT - for key in _DATE_RANGE_PARAMS: - if key in kwargs: - kwargs[key] = _format_api_dates( - kwargs[key], - name=key, - date=( - collection in dialect.date_only_services and key != "last_modified" - ), - open_start=dialect.open_start, - ) + for key in dialect.date_params.intersection(kwargs): + kwargs[key] = _write_date_param(kwargs[key], key, collection, dialect) json_filter = _pop_json_filter(kwargs) params, post_params = _partition_request_params( kwargs, use_cql2=dialect.lists_as_cql2 @@ -276,16 +282,15 @@ def _construct_cql_request( # --------------------------------------------------------------------------- # Iterable params that ``_get_args`` must not pass through -# ``_normalize_str_iterable`` (date-range params may contain ``pd.NaT``/None or -# interval strings; ``bbox`` is ``list[float]``). These apply to every OGC caller; -# an adapter with extra numeric params names only its extras via +# ``_normalize_str_iterable`` (``bbox`` is ``list[float]``). These apply to every +# OGC caller; an adapter with extra numeric params names only its extras via # ``prepare_request_args(..., extra_no_normalize=...)``. -_NO_NORMALIZE_PARAMS = _DATE_RANGE_PARAMS | {"bbox"} +_NO_NORMALIZE_PARAMS = frozenset({"bbox"}) def _normalize_str_iterable( value: str | Iterable[str] | None, - param_name: str = "value", + param_name: str, ) -> str | list[str] | None: """Validate that ``value`` is None, a string, or an iterable of strings.""" if value is None: @@ -310,7 +315,7 @@ def _normalize_str_iterable( def _as_str_list( value: str | Iterable[str] | None, - param_name: str = "value", + param_name: str, ) -> list[str] | None: """Normalize ``value`` to ``list[str]`` (``None`` passes through).""" normalized = _normalize_str_iterable(value, param_name) @@ -342,16 +347,50 @@ def _check_monitoring_location_id( return value +def _read_date(key: str, value: Any, dialect: OgcDialect) -> DateRange | None: + """A date argument as a ``DateRange``, read with the dialect of the API it + is sent to; ``None`` when every bound is open.""" + if isinstance(value, DateRange): + return value + return read_date_range(value, name=key, durations=dialect.dates.durations) + + +def _read_raw_dates(args: dict[str, Any], dialect: OgcDialect) -> dict[str, Any]: + """``args`` with every date parameter of ``dialect`` read, including a + verbatim ``query`` that did not go through :func:`prepare_request_args`. + + A ``DateRange`` under a key the dialect does not list would be sent as its + repr, so it raises: the arguments were read with another API's dialect. + """ + out: dict[str, Any] = {} + for key, value in args.items(): + if key in dialect.date_params: + value = _read_date(key, value, dialect) + if value is None: + continue + elif isinstance(value, DateRange): + raise TypeError( + f"{key} was read as a date range, but this API's dialect has no " + f"date parameter {key!r}. Read the arguments with the dialect " + "of the API they are sent to." + ) + out[key] = value + return out + + def _normalize_arg( key: str, value: Any, no_normalize: frozenset[str], + dialect: OgcDialect | None, ) -> Any: """Normalize a single request argument value based on its key.""" if key == "monitoring_location_id": return _check_monitoring_location_id(value) if key == "properties": return _as_str_list(value, key) + if dialect is not None and key in dialect.date_params: + return _read_date(key, value, dialect) if ( key in no_normalize and isinstance(value, Iterable) @@ -367,18 +406,21 @@ def prepare_request_args( local_vars: dict[str, Any], exclude: set[str] | None = None, *, + dialect: OgcDialect | None = None, extra_no_normalize: frozenset[str] | set[str] = frozenset(), ) -> dict[str, Any]: - """Build OGC request kwargs from a getter's ``locals()``. + """Build request kwargs from a getter's ``locals()``. Internal control keys, caller-supplied exclusions, and ``None`` values - are omitted. Identifiers and properties are validated; other iterables are - normalized unless exempted. + are omitted. Identifiers and properties are validated, the date parameters + of ``dialect`` are read into a ``DateRange`` (and omitted when every bound + is open), and other iterables are normalized unless exempted. Without a + dialect no argument is read as a date, as for an API that is not OGC. ``extra_no_normalize`` *adds* to the engine's own :data:`_NO_NORMALIZE_PARAMS` rather than replacing it, so an adapter names - only the params it owns and cannot drop the date-range exemptions - by omitting them. + only the params it owns and cannot drop the ``bbox`` exemption by + omitting it. """ no_normalize = _NO_NORMALIZE_PARAMS | frozenset(extra_no_normalize) to_exclude = {"collection", "service", "output_id"} @@ -387,7 +429,9 @@ def prepare_request_args( args: dict[str, Any] = {} for k, v in local_vars.items(): - if k in to_exclude or v is None: + if k in to_exclude: continue - args[k] = _normalize_arg(k, v, no_normalize) + normalized = _normalize_arg(k, v, no_normalize, dialect) + if normalized is not None: + args[k] = normalized return args diff --git a/dataretrieval/waterdata/ratings.py b/dataretrieval/waterdata/ratings.py index ebeb0139..cc94ea61 100644 --- a/dataretrieval/waterdata/ratings.py +++ b/dataretrieval/waterdata/ratings.py @@ -18,9 +18,10 @@ import httpx import pandas as pd +from dataretrieval._dates import read_date_range +from dataretrieval._intervals import DateDialect, write_interval from dataretrieval._validation import render_options from dataretrieval.exceptions import DataRetrievalError, SkippedRatingWarning -from dataretrieval.ogc.dates import _DURATION_RE, _coerce_to_list, _format_api_dates from dataretrieval.ogc.errors import _raise_for_non_200 from dataretrieval.ogc.filters import _cql_in_clause from dataretrieval.ogc.requests import _check_monitoring_location_id @@ -40,6 +41,11 @@ RATING_FILE_TYPE = Literal["exsa", "base", "corr"] _VALID_FILE_TYPES = get_args(RATING_FILE_TYPE) +# The STAC firewall rejects ``datetime=../2026-01-01`` with HTTP 403, and STAC +# treats an empty start (``/2026-01-01T00:00:00Z``) as open. A lone duration +# (``datetime=P7D``) is rejected with HTTP 400, the standard's default. +STAC_DATES = DateDialect(open_start="") + def get_ratings( monitoring_location_id: str | Iterable[str] | None = None, @@ -177,18 +183,11 @@ def get_ratings( monitoring_location_id = _check_monitoring_location_id(monitoring_location_id) file_types = _as_list(file_type) _validate_file_types(file_types) - _validate_time_no_duration(time) + time_range = read_date_range(time, name="time", durations=STAC_DATES.durations) time_str = ( - _format_api_dates( - time, - name="time", - single_value_hint="an instant ('2020-01-01')", - # The STAC firewall rejects ``datetime=../2026-01-01`` with HTTP - # 403; STAC treats an empty start (``/2026-01-01``) as open. - open_start="", - ) - if time is not None - else None + None + if time_range is None + else write_interval(time_range, STAC_DATES, date_only=False) ) # Match R: set file_type server-side only when one type is requested. @@ -223,20 +222,6 @@ def _validate_file_types(file_types: list[str]) -> None: ) -def _validate_time_no_duration(time: str | list[str] | None) -> None: - """Raise ValueError if ``time`` contains an ISO 8601 duration.""" - if time is None: - return - # ``_coerce_to_list`` keeps a lone ``date``/``Timestamp`` whole, where - # ``_as_list`` would try to iterate it. - values = _coerce_to_list(time, "time") - if any(isinstance(v, str) and _DURATION_RE.match(v) for v in values): - raise ValueError( - "ISO 8601 durations (e.g. 'P7D') are not supported in `time` " - "for the rating-curve service. Provide a date or interval instead." - ) - - def _filter_features_by_type( features: list[dict[str, Any]], file_types: list[str] ) -> list[dict[str, Any]]: diff --git a/dataretrieval/waterdata/samples.py b/dataretrieval/waterdata/samples.py index 7e70f6a2..bf957ec6 100644 --- a/dataretrieval/waterdata/samples.py +++ b/dataretrieval/waterdata/samples.py @@ -356,7 +356,7 @@ def get_samples( # Build argument dictionary, omitting None values. Parameters are the # public snake_case names here; translate them to the camelCase names the # Samples API expects just before building the request. - args = _get_args(locals(), exclude={"ssl_check", "profile"}) + args = _get_args(locals(), exclude={"ssl_check", "profile"}, dialect=None) params = {_SAMPLES_PARAM_TO_API.get(key, key): value for key, value in args.items()} params.update({"mimeType": "text/csv"}) diff --git a/dataretrieval/waterdata/time_series.py b/dataretrieval/waterdata/time_series.py index 96b2ff9f..06dc0f65 100644 --- a/dataretrieval/waterdata/time_series.py +++ b/dataretrieval/waterdata/time_series.py @@ -1044,6 +1044,7 @@ def get_stats_por( params = _get_args( _with_state(locals(), to="fips_us", into="state_code"), exclude={"expand_percentiles"}, + dialect=None, ) return stats.get_data( @@ -1191,6 +1192,7 @@ def get_stats_date_range( params = _get_args( _with_state(locals(), to="fips_us", into="state_code"), exclude={"expand_percentiles"}, + dialect=None, ) return stats.get_data( diff --git a/dataretrieval/waterdata/utils.py b/dataretrieval/waterdata/utils.py index 3fe6b829..1307b273 100644 --- a/dataretrieval/waterdata/utils.py +++ b/dataretrieval/waterdata/utils.py @@ -23,6 +23,7 @@ import pandas as pd from dataretrieval._deprecation import warn_deprecated +from dataretrieval._intervals import DateDialect from dataretrieval.codes.states import apply_state from dataretrieval.credentials import refuse_credential_keywords from dataretrieval.ogc import OgcDialect, prepare_request_args @@ -82,9 +83,16 @@ # ``construction_date`` is deliberately not a time column: the service sends it # as a string at day, month, or year precision (``19950812``, ``199508``, # ``2005``), and no one datetime parse keeps all three, so it stays as sent. +# The API accepts a lone duration (``time=P7D``), which the standard does not +# define, and rejects an empty open start (``time=/2024-01-31``, HTTP 400). WATERDATA_DIALECT = OgcDialect( lists_as_cql2=True, - date_only_services=frozenset({"daily"}), + date_params=frozenset( + {"datetime", "last_modified", "begin", "begin_utc", "end", "end_utc", "time"} + ), + date_only_collections=frozenset({"daily"}), + timed_params=frozenset({"last_modified"}), + dates=DateDialect(durations=True), time_cols=frozenset( { "begin", @@ -170,10 +178,16 @@ def _flatten_queryables(local_vars: dict[str, Any]) -> dict[str, Any]: def _get_args( - local_vars: dict[str, Any], exclude: set[str] | None = None + local_vars: dict[str, Any], + exclude: set[str] | None = None, + *, + dialect: OgcDialect | None = WATERDATA_DIALECT, ) -> dict[str, Any]: """Water-Data wrapper over :func:`~dataretrieval.ogc.prepare_request_args`. + Date parameters are read with ``dialect``, the OGC API's by default; the + Samples and Statistics getters pass ``None``, since those APIs are not OGC. + Adds the Water Data API's extra no-normalize params (numeric params such as ``water_year``, ``thresholds``, ``boundingBox``) so they keep their element types. Also flattens any ``**queryables`` passthrough (see @@ -181,7 +195,10 @@ def _get_args( """ _flatten_queryables(local_vars) args = prepare_request_args( - local_vars, exclude, extra_no_normalize=_NO_NORMALIZE_PARAMS + local_vars, + exclude, + dialect=dialect, + extra_no_normalize=_NO_NORMALIZE_PARAMS, ) for name in _JSON_ARRAY_PARAMS & args.keys(): # An empty list is dropped from the request downstream, as for any filter. diff --git a/docs/source/architecture/decisions/0014-services-endpoints-dialects.rst b/docs/source/architecture/decisions/0014-services-endpoints-dialects.rst new file mode 100644 index 00000000..5ee73580 --- /dev/null +++ b/docs/source/architecture/decisions/0014-services-endpoints-dialects.rst @@ -0,0 +1,169 @@ +ADR 0014: Declare how each service departs from its protocol as data +==================================================================== + +Status +------ + +Proposed. Implemented in three pull requests, listed under ``Notes``. It is +accepted when the last one merges. + +Context +------- + +Water Data and NGWMN share the OGC API Features code in ``dataretrieval.ogc``, +and Water Data's ratings catalog uses STAC, which writes dates in the same ISO +8601 interval grammar. The three services do not handle the standards the same +way: + +.. list-table:: + :header-rows: 1 + + * - Request (observed 2026-10-11) + - Water Data OGC + - NGWMN + - Water Data STAC + * - A lone duration, ``P7D`` + - accepted + - HTTP 400 + - HTTP 400 + * - An open start, ``../2016-01-01`` + - accepted + - HTTP 403 from the firewall + - HTTP 403 from the firewall + * - An empty start, ``/2016-01-01`` + - HTTP 400 (500 on ``continuous``) + - accepted + - accepted + * - A comma-joined list, ``site=A,B`` + - read as one value, except for a few identifiers and codes + - read as a list + - not used + +The shared code handles these differences in two ways. Some are fields of +``OgcDialect``, which each adapter sets: ``lists_as_cql2`` (#446) and the open +start (#443) were added that way while fixing bugs. Others are written into the +shared code: it lists Water Data's date parameter names, exempts +``last_modified`` by name from date-only writing, passes a duration through +for every service because Water Data accepts one, and passes each adapter's +name as a string literal to configuration, retry, and fan-out. ADR 0003 keeps +shared code from *importing* a service; it does not stop shared code from +*naming* one. + +Dates were also read and written in one function. Reading what a caller passed +(a string, a ``date``, a ``Timestamp``, ``None``, the time-zone rule) is +package policy; writing it as an ISO 8601 interval is protocol grammar, and +differs per service. Each of the date defects fixed in #440 through #443 was in +that one function. + +Decision +-------- + +**A service's departure from its protocol's standard is a dialect field.** A +dialect is a frozen dataclass with one type per protocol. Each field records +one way a service extends the standard (Water Data accepts a lone duration), +restricts it (NGWMN rejects ``../``), or renames it (Water Data filters dates on +``time``). Each field's default is the standard, so a default dialect describes +a service that follows the standard. Where an adapter sets a field, a comment +records the observed server behavior that requires it, and a live test sends +both the request the field prevents and the request it produces. + +**Shared code names no service.** A parameter, collection, or column that a +dialect declares does not appear as a literal in protocol code, and a service +name does not appear as a literal outside the configuration roster and the +service's own declaration. A protocol's own standard names (``datetime``, +``bbox``, ``limit``) are not departures. A comment may cite a service as the +reason the code accepts a variation. + +**A date argument is read once and written by each protocol.** +``dataretrieval._dates`` reads a date argument into a ``DateRange``: an +instant, a range whose ``None`` bound is open, or a duration, with every bound +in UTC (a naive value is UTC, as in pandas). ``dataretrieval._intervals`` +writes a ``DateRange`` in the ISO 8601 interval grammar, adjusted by a +``DateDialect`` (``open_start``, ``durations``). It sits below ``ogc`` because +STAC also uses it. + +**Values are data; protocol code is functions.** Dialects, and the ``Service`` +and ``Endpoint`` values below, are frozen dataclasses with no methods that send +requests or read configuration, and nothing subclasses them. A method or an +override could change one service's behavior without adding a field. + +**The OGC engine receives one endpoint value.** An ``Endpoint`` holds a +``Service`` (the adapter's name, which configuration resolves settings by, and +its default URL), the URL resolved when the call starts, and the dialect. +``get_ogc_data`` takes it in place of the ``base_url``, ``dialect``, and +adapter-name arguments it takes today, so every chunk and a later resume use +the same URL and dialect. The request builder and result finalizer it binds +keep taking the URL and dialect they read. + +**A protocol module exists only when two services share the protocol.** OGC +API Features has two (Water Data, NGWMN); the interval grammar has three (those +two and STAC). WQP, NLDI, NWDC, StreamStats, and Water Data's Samples and +Statistics APIs keep their request code in their adapters, and get no dialect +or endpoint value until a second service shares their protocol. + +Consequences +------------ + +- Adding an OGC or STAC service means setting dialect fields, not editing + shared code. A service that follows the standard needs none. +- What the shared code knows about a service is in one declaration, next to the + evidence for each field, and the scheduled live workflow fails when a + service's behavior changes. +- A departure that the protocol code does not model needs a new field and + default before an adapter can use it. This is deliberate: it keeps the + departure visible and tested. +- Behavior change: a lone duration passed to NGWMN or to ``get_ratings`` + raises ``ValueError`` before sending, instead of returning HTTP 400 from + NGWMN; ``get_ratings`` already raised, with its own message. + +Alternatives rejected: + +- *Conditionals on the service name in shared code.* This is how the + differences listed above were handled before, and reviews missed the cases + where a change for one service broke another. +- *A client class per service, with methods that send requests.* Behavior + behind a method can be overridden for one service without a field, and the + module that defines the dialect would have to import the OGC engine. +- *One flat dialect.* STAC reads the date fields without the rest. +- *Probing a service at runtime for what it accepts.* That adds a request per + call to learn facts that change rarely, and some come from a firewall the API + does not describe. + +Compliance +---------- + +- ``.importlinter`` places ``_dates`` with the dependency-free leaves and + ``_intervals`` below ``ogc``. +- Each ``DateDialect`` field an adapter sets has a ``@pytest.mark.live`` test + in that adapter's test file. +- From the second step, ``tests/architecture_test.py`` fails if a string a + shipped dialect lists appears as a literal in ``ogc`` or ``_intervals``; + from the third, if a service name appears as a literal outside its + declaration and the configuration roster. Docstrings and comments are + excluded. + +Departures still written into the OGC code, each removed when another service +needs a different value or the code holding it next changes: + +- ``monitoring_location_id`` is validated against the ``AGENCY-ID`` form both + OGC services use. +- ``skipGeometry`` is pygeoapi's name; the standard has no such parameter. +- ``limit`` is capped at 50000, Water Data's largest page. +- An unquoted number in a CQL text filter raises, because every Water Data + queryable is typed as a string. + +Notes +----- + +Prompted by the reviews of #442 and #443. Replaces the draft in #445, which +implemented every step in one branch, and the narrower #444. + +Steps, one pull request each: + +1. The date split: ``_dates``, ``_intervals``, and ``DateDialect``, with the + duration refusal for NGWMN and STAC. +2. The OGC date fields on ``OgcDialect`` (which parameters carry dates, which + collections are date-only, which parameters stay timed), reading dates when + the getter is called, and the dialect-literal test. +3. ``Service`` declarations for every adapter, ``Endpoint`` for + ``get_ogc_data``, and the service-name test. diff --git a/docs/source/architecture/decisions/index.rst b/docs/source/architecture/decisions/index.rst index 29825505..104ad58a 100644 --- a/docs/source/architecture/decisions/index.rst +++ b/docs/source/architecture/decisions/index.rst @@ -32,4 +32,5 @@ belong here rather than in a docstring, an inline comment, or a commit message. 0011-configuration-profiles 0012-deprecation-horizons 0013-core-and-domain-terms + 0014-services-endpoints-dialects template diff --git a/docs/source/architecture/index.rst b/docs/source/architecture/index.rst index a71518f0..6c0ae981 100644 --- a/docs/source/architecture/index.rst +++ b/docs/source/architecture/index.rst @@ -118,10 +118,11 @@ Shared components ``planning`` determines chunk boundaries; ``chunking`` connects those plans to the shared fan-out executor and retains compatibility aliases; ``interruptions`` retains its deprecated public import path; and - ``shaping``, ``dates``, ``filters``, and ``errors`` isolate their named - protocol concerns. The full runtime OGC graph, including the facade, is - acyclic -- enforced by the package-wide fitness function in - ``tests/architecture_test.py``. + ``shaping``, ``filters``, and ``errors`` isolate their named protocol + concerns. Date parameters are written by ``dataretrieval._intervals``, the + interval grammar OGC shares with STAC (ADR 0014). The full runtime OGC + graph, including the facade, is acyclic -- enforced by the package-wide + fitness function in ``tests/architecture_test.py``. ``dataretrieval.transport`` Internal service-neutral execution layer. Owns guarded client lifecycle and diff --git a/tests/_dates_test.py b/tests/_dates_test.py new file mode 100644 index 00000000..7fe0993d --- /dev/null +++ b/tests/_dates_test.py @@ -0,0 +1,69 @@ +"""Reading a date argument into a ``DateRange`` (ADR 0014). + +How each shape is written for the wire is tested with the OGC request builder +in ``waterdata_utils_test.py``. +""" + +import datetime + +import pandas as pd +import pytest + +from dataretrieval._dates import DateRange, read_date_range +from dataretrieval.ogc import prepare_request_args +from dataretrieval.ogc.policy import OgcDialect +from dataretrieval.ogc.requests import _read_raw_dates + +UTC = datetime.datetime(2024, 1, 1, tzinfo=datetime.timezone.utc) + + +@pytest.mark.parametrize( + "value, expected", + [ + ("2024-01-01", DateRange(start=UTC, instant=True)), + (datetime.date(2024, 1, 1), DateRange(start=UTC, instant=True)), + ("P7D", DateRange(duration="P7D")), + (["2024-01-01T00:00:00Z", None], DateRange(start=UTC)), + ("../2024-01-01", DateRange(end=UTC)), + ([pd.NaT, pd.Timestamp("2024-01-01")], DateRange(end=UTC)), + ("2023-12-31T18:00:00-06:00", DateRange(start=UTC, instant=True)), + ], + ids=["string", "date", "duration", "open_end", "open_start", "nat", "offset"], +) +def test_read_date_range_keeps_the_shape_the_caller_passed(value, expected): + """An instant, a duration, and a range with open bounds stay distinct, and + every bound is UTC: a naive one is read as UTC, an aware one converted.""" + assert read_date_range(value, name="time", durations=True) == expected + + +@pytest.mark.parametrize("value", [None, pd.NaT, [None, ".."], "../.."]) +def test_read_date_range_returns_none_for_no_filter(value): + assert read_date_range(value, name="time", durations=True) is None + + +def test_prepare_request_args_reads_date_arguments_on_entry(): + """A getter's date arguments are read when it is called, so an unreadable + one raises before any request is planned, and an all-open one is dropped + like ``None``.""" + dialect = OgcDialect(date_params=frozenset({"time", "last_modified"})) + args = prepare_request_args( + {"time": ["2024-01-01", None], "last_modified": [None, ".."]}, + dialect=dialect, + ) + assert args == {"time": DateRange(start=UTC)} + with pytest.raises(ValueError, match="^time could not be read"): + prepare_request_args({"time": "yesterday"}, dialect=dialect) + + +def test_prepare_request_args_reads_no_date_without_a_dialect(): + """An API that is not OGC (Samples) gets its arguments as given.""" + assert prepare_request_args({"time": "2024-01-01"}) == {"time": "2024-01-01"} + + +def test_a_date_range_the_dialect_does_not_list_raises(): + """Arguments read with one dialect and sent with another would put a + ``DateRange`` repr in the URL.""" + args = {"time": DateRange(start=UTC)} + assert _read_raw_dates(args, OgcDialect(date_params=frozenset({"time"}))) == args + with pytest.raises(TypeError, match="^time was read as a date range"): + _read_raw_dates(args, OgcDialect()) diff --git a/tests/architecture_test.py b/tests/architecture_test.py index c098eb4d..caf89f3b 100644 --- a/tests/architecture_test.py +++ b/tests/architecture_test.py @@ -27,6 +27,7 @@ import ast import configparser +import dataclasses import functools import re import sys @@ -667,3 +668,87 @@ def test_adr_references_resolve_to_a_record() -> None: assert not dangling, "citations name no such decision record:\n " + "\n ".join( dangling ) + + +def _code_string_literals(path: Path) -> list[tuple[int, str]]: + """String constants in *path*, excluding docstrings and ``__all__``, whose + entries name modules and symbols.""" + tree = ast.parse(path.read_text(encoding="utf-8")) + exports = { + id(element) + for node in ast.walk(tree) + if isinstance(node, ast.Assign) + and any(isinstance(t, ast.Name) and t.id == "__all__" for t in node.targets) + for element in ast.walk(node.value) + } + docstrings = exports | { + id(node.body[0].value) + for node in ast.walk(tree) + if isinstance( + node, (ast.Module, ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef) + ) + and node.body + and isinstance(node.body[0], ast.Expr) + and isinstance(node.body[0].value, ast.Constant) + } + return [ + (node.lineno, node.value) + for node in ast.walk(tree) + if isinstance(node, ast.Constant) + and isinstance(node.value, str) + and id(node) not in docstrings + ] + + +# Protocol code: the modules shared by more than one service (ADR 0014). +_PROTOCOL_PATHS = ( + *sorted((PACKAGE_ROOT / "ogc").glob("*.py")), + PACKAGE_ROOT / "_intervals.py", +) + +# The departures ADR 0014 records as still written in the OGC code. Only this +# one is a literal a dialect also lists; the other three (the ``skipGeometry`` +# name, the 50000 ``limit`` cap, and the unquoted-number CQL check) name +# nothing a dialect declares. +_RECORDED_DEPARTURES = frozenset({"monitoring_location_id"}) + + +def _declared_strings(value: object) -> set[str]: + """Every string a dialect value lists, through nested dialects.""" + if dataclasses.is_dataclass(value) and not isinstance(value, type): + return set().union( + *( + _declared_strings(getattr(value, f.name)) + for f in dataclasses.fields(value) + ) + ) + if isinstance(value, str): + return {value} if value else set() + if isinstance(value, (frozenset, set, tuple, list)): + return set().union(*(_declared_strings(v) for v in value)) + return set() + + +def test_protocol_code_names_nothing_a_dialect_declares() -> None: + """ADR 0014: shared protocol code reads every parameter, collection, and + column a service departs on from that service's dialect. A literal of one + in protocol code is behavior that depends on which service it is, without + a field saying so. The standard's own names (each field's default) are not + departures.""" + from dataretrieval.ngwmn import NGWMN_DIALECT + from dataretrieval.ogc import OgcDialect + from dataretrieval.waterdata.ratings import STAC_DATES + from dataretrieval.waterdata.utils import WATERDATA_DIALECT + + declared = ( + _declared_strings(WATERDATA_DIALECT) + | _declared_strings(NGWMN_DIALECT) + | _declared_strings(STAC_DATES) + ) - _declared_strings(OgcDialect()) + found = [ + f"{path.relative_to(PACKAGE_ROOT)}:{line} {value!r}" + for path in _PROTOCOL_PATHS + for line, value in _code_string_literals(path) + if value in declared - _RECORDED_DEPARTURES + ] + assert not found, f"declare it as a dialect field and read it there: {found}" diff --git a/tests/conftest.py b/tests/conftest.py index 793dfb06..1c773096 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -64,3 +64,18 @@ def _pin_chunker_env(monkeypatch, tmp_path): # pointing the variable at a file they wrote. monkeypatch.setenv("DATARETRIEVAL_CONFIG", str(tmp_path / "no-such-config.toml")) configuration._reset_file_cache() + + +@pytest.fixture +def live_status(): + """``live_status(url, **params)``: the status a running service returns for + a GET, for the live tests that check a dialect field against the server.""" + import httpx + + from dataretrieval.transport.http import default_headers + + def status(url: str, **params: object) -> int: + headers = default_headers(url) + return httpx.get(url, params=params, headers=headers, timeout=60).status_code + + return status diff --git a/tests/ngwmn_test.py b/tests/ngwmn_test.py index a0cf4a69..c907c8dc 100644 --- a/tests/ngwmn_test.py +++ b/tests/ngwmn_test.py @@ -472,6 +472,39 @@ def test_get_water_level_sends_an_open_start_empty(httpx_mock, datetime): assert qs["datetime"] == ["/2016-01-01T00:00:00Z"] +def test_get_water_level_rejects_a_duration_before_any_request(httpx_mock): + """NGWMN rejects a lone duration with HTTP 400, so it raises before sending.""" + with pytest.raises(ValueError, match="^datetime cannot be an ISO 8601 duration"): + ngwmn.get_water_level(monitoring_location_id=_SITE, datetime="P7D") + assert httpx_mock.get_requests() == [] + + +@pytest.mark.live +def test_date_dialect_matches_the_service(live_status): + """Each ``NGWMN_DIALECT.dates`` field against the running service: the + request the field prevents fails, and the one it produces succeeds. Kept + live because the 403 comes from a firewall the API does not describe.""" + import datetime + + from dataretrieval._dates import DateRange + from dataretrieval._intervals import write_interval + from dataretrieval.ngwmn import NGWMN_DIALECT, NGWMN_OGC_API_URL + + url = f"{NGWMN_OGC_API_URL}/collections/waterLevelObs/items" + + def status(value): + return live_status( + url, monitoring_location_id=_OTHER_SITE, limit=1, datetime=value + ) + + # open_start: "../" is rejected; the empty start is treated as open. + end = DateRange(end=datetime.datetime(2016, 1, 1, tzinfo=datetime.timezone.utc)) + assert status("../2016-01-01T00:00:00Z") == 403 + assert status(write_interval(end, NGWMN_DIALECT.dates, date_only=False)) == 200 + # durations=False: a lone duration is rejected, so none is sent. + assert status("P7D") == 400 + + def test_get_lithology(httpx_mock): _mock(httpx_mock, "lithologyObs", _LITHOLOGY) diff --git a/tests/waterdata_chunking_test.py b/tests/waterdata_chunking_test.py index b9689ee7..ac2b737d 100644 --- a/tests/waterdata_chunking_test.py +++ b/tests/waterdata_chunking_test.py @@ -34,6 +34,7 @@ import dataretrieval from dataretrieval import configuration as _configuration +from dataretrieval._dates import read_date_range from dataretrieval.combining import ( _QUOTA_HEADER, _combine_chunk_frames, @@ -62,10 +63,8 @@ multi_value_chunked, parallel_chunks, ) -from dataretrieval.ogc.dates import _DATE_RANGE_PARAMS from dataretrieval.ogc.planning import ( _LIST_SEP, - _NEVER_CHUNK, _OR_SEP, ChunkPlan, _extract_axes, @@ -148,19 +147,17 @@ def _fake_build(*, base=200, **kwargs): return _FakeReq("x" * bytes_) -def test_never_chunk_covers_all_date_range_params(): - """``_NEVER_CHUNK`` and ``_DATE_RANGE_PARAMS`` are maintained in - separate modules (chunker vs request builder) for layering reasons, - but every date-range param MUST be excluded from chunking — a - range value isn't an enumerable set to split. Guard against drift: - adding a new param to ``_DATE_RANGE_PARAMS`` without also adding - it to ``_NEVER_CHUNK`` would let the chunker try to - comma-join an interval string.""" - missing = _DATE_RANGE_PARAMS - _NEVER_CHUNK - assert not missing, ( - f"_DATE_RANGE_PARAMS contains entries not in _NEVER_CHUNK: " - f"{sorted(missing)}. Add them to planning._NEVER_CHUNK." - ) +def test_extract_axes_never_splits_a_date_range(): + """A date argument arrives as a ``DateRange``, so it is never an axis, + whatever the dialect names the parameter -- the planner lists no date + parameter of its own.""" + args = { + "monitoring_location_id": ["USGS-A", "USGS-B"], + "any_date_param": read_date_range( + ["2024-01-01", "2024-12-31"], name="time", durations=True + ), + } + assert [axis.arg_key for axis in _extract_axes(args)] == ["monitoring_location_id"] def test_extract_axes_picks_up_list_dims_and_filter(): diff --git a/tests/waterdata_ratings_test.py b/tests/waterdata_ratings_test.py index 4296c5cc..22dbb67d 100644 --- a/tests/waterdata_ratings_test.py +++ b/tests/waterdata_ratings_test.py @@ -45,13 +45,38 @@ def test_get_ratings_rejects_invalid_file_type(): def test_get_ratings_rejects_iso_8601_duration_in_time(): """STAC ratings doesn't accept ISO 8601 durations; raise a clear error.""" - with pytest.raises(ValueError, match=r"durations.*not supported"): + with pytest.raises(ValueError, match=r"^time cannot be an ISO 8601 duration"): get_ratings( monitoring_location_id="USGS-01104475", time="P7D", ) +@pytest.mark.live +def test_stac_date_dialect_matches_the_service(live_status): + """Each ``STAC_DATES`` field against the running search: the request the + field prevents fails, and the one it produces succeeds. Kept live because + the 403 comes from a firewall the API does not describe.""" + import datetime + + from dataretrieval._dates import DateRange + from dataretrieval._intervals import write_interval + from dataretrieval.waterdata.endpoints import ratings_catalog_url + from dataretrieval.waterdata.ratings import STAC_DATES + + url = f"{ratings_catalog_url()}/search" + + def status(value): + return live_status(url, limit=1, datetime=value) + + # open_start: "../" is rejected; the empty start is treated as open. + end = DateRange(end=datetime.datetime(2026, 1, 1, tzinfo=datetime.timezone.utc)) + assert status("../2026-01-01T00:00:00Z") == 403 + assert status(write_interval(end, STAC_DATES, date_only=False)) == 200 + # durations=False: a lone duration is rejected, so none is sent. + assert status("P7D") == 400 + + def test_build_filter_escapes_quotes(): """Prevents malformed CQL or injection if an ID contains a quote.""" f = _build_filter("USGS-x'-y", None) @@ -216,8 +241,8 @@ def test_get_ratings_keeps_a_dotdot_open_bound_in_time(httpx_mock): ids=["lone", "range"], ) def test_get_ratings_accepts_a_timestamp_time(httpx_mock, time): - """The duration check iterated a lone ``Timestamp`` and raised - ``TypeError`` before the date formatter could read it.""" + """A lone ``Timestamp`` used to raise ``TypeError`` in a separate duration + check that iterated it before the date was read.""" httpx_mock.add_response( method="GET", url=STAC_SEARCH_RE, json=_stub_search_response() ) diff --git a/tests/waterdata_test.py b/tests/waterdata_test.py index 4d282636..de596858 100644 --- a/tests/waterdata_test.py +++ b/tests/waterdata_test.py @@ -730,6 +730,32 @@ def test_construct_api_requests_keeps_dotdot_for_an_open_start(): assert "time=..%2F2024-01-31" in str(req.url) +@pytest.mark.live +def test_ogc_date_dialect_matches_the_service(live_status): + """Each ``WATERDATA_DIALECT.dates`` field against the running API: it + accepts the request each field produces and rejects the empty start.""" + from dataretrieval._dates import DateRange + from dataretrieval._intervals import write_interval + from dataretrieval.waterdata.endpoints import ogc_api_url + + url = f"{ogc_api_url()}/collections/daily/items" + dates = WATERDATA_DIALECT.dates + + def status(value): + return live_status( + url, monitoring_location_id="USGS-05427718", limit=1, time=value + ) + + # open_start: the default ".." is accepted; the empty start is rejected. + end = DateRange(end=datetime.datetime(2024, 1, 3, tzinfo=datetime.timezone.utc)) + assert status(write_interval(end, dates, date_only=True)) == 200 + assert status("/2024-01-03") == 400 + # durations=True: a lone duration is accepted, so it is sent. + assert ( + status(write_interval(DateRange(duration="P7D"), dates, date_only=False)) == 200 + ) + + 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` @@ -1722,6 +1748,32 @@ def test_get_reference_table_serves_countries(httpx_mock): assert "country" in df.columns +@pytest.mark.parametrize( + ("value", "sent"), + [ + ("2024-01-01T00:00:00Z", "2024-01-01T00:00:00Z"), + (["2024-01-01T00:00:00Z", None], "2024-01-01T00:00:00Z/.."), + ], +) +def test_get_reference_table_reads_a_date_in_its_query(httpx_mock, value, sent): + """A ``query`` dict skips the getters' argument reading, so the engine reads + its date parameters.""" + _mock_items(httpx_mock, "agency-codes") + + get_reference_table("agency-codes", query={"last_modified": value}) + + assert _sent(httpx_mock, "agency-codes")[0]["last_modified"] == [sent] + + +def test_get_reference_table_drops_an_all_open_date_in_its_query(httpx_mock): + """``[None, None]`` means no date filter, in a ``query`` as in a getter.""" + _mock_items(httpx_mock, "agency-codes") + + get_reference_table("agency-codes", query={"last_modified": [None, None]}) + + assert "last_modified" not in _sent(httpx_mock, "agency-codes")[0] + + def test_get_reference_table_with_query(httpx_mock): """A ``query`` dict is merged into the request's query params.""" _mock_items(httpx_mock, "agency-codes") diff --git a/tests/waterdata_utils_test.py b/tests/waterdata_utils_test.py index 9348ffbe..64045f49 100644 --- a/tests/waterdata_utils_test.py +++ b/tests/waterdata_utils_test.py @@ -14,6 +14,8 @@ import dataretrieval.ogc.shaping as _shaping_module import dataretrieval.waterdata.stats as _stats_module import dataretrieval.waterdata.utils as _utils_module +from dataretrieval._dates import read_date_range +from dataretrieval._intervals import DateDialect, write_interval from dataretrieval.exceptions import ( DataRetrievalError, HTTPError, @@ -22,7 +24,6 @@ TransientError, ) from dataretrieval.interruptions import ServiceInterrupted -from dataretrieval.ogc.dates import _format_api_dates from dataretrieval.ogc.engine import ( _next_req_url, _walk_pages, @@ -66,6 +67,14 @@ _LOGGER_NAME = _utils_module.__name__ +def _format_api_dates(value, date=False, *, name="date input", open_start=".."): + """Read a date argument and write it as an API that accepts durations + would receive it.""" + dialect = DateDialect(open_start=open_start, durations=True) + read = read_date_range(value, name=name, durations=dialect.durations) + return None if read is None else write_interval(read, dialect, date_only=date) + + def _run_walk_pages(*, geopd, req, client, row_cap=None): """Run the async ``_walk_pages`` to completion synchronously. @@ -1131,20 +1140,30 @@ def test_format_api_dates_rejects_more_than_two_values(): def test_the_duration_example_is_withheld_where_durations_are_rejected(): - """``get_ratings`` refuses ISO 8601 durations, so the shared message must not offer - 'P7D' on that path -- a caller following it would receive a second rejection.""" + """An API that refuses ISO 8601 durations must not be offered 'P7D' in the + message -- a caller following it would receive a second rejection.""" with pytest.raises(ValueError) as allowed: - _format_api_dates(["a", "b", "c"], name="time") + read_date_range(["a", "b", "c"], name="time", durations=True) with pytest.raises(ValueError) as refused: - _format_api_dates( - ["a", "b", "c"], name="time", single_value_hint="an instant ('2020-01-01')" - ) + read_date_range(["a", "b", "c"], name="time", durations=False) assert "'P7D'" in str(allowed.value) assert "'P7D'" not in str(refused.value) assert "an instant ('2020-01-01')" in str(refused.value) +@pytest.mark.parametrize("value", ["P7D", ["PT36H"]], ids=["string", "list"]) +def test_a_duration_raises_where_the_api_refuses_one(value): + """Neither standard defines a lone duration, so an API whose dialect does not + accept one raises before any request rather than receiving HTTP 400.""" + with pytest.raises(ValueError, match="^time cannot be an ISO 8601 duration"): + read_date_range(value, name="time", durations=False) + assert read_date_range(value, name="time", durations=True).duration in { + "P7D", + "PT36H", + } + + def test_format_api_dates_names_the_callers_parameter(): """The subject must be the argument the caller passed. It used to be ``datetime_input`` -- a local of this private helper that no public getter