diff --git a/.importlinter b/.importlinter index 7dc23e52..473e3b40 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 a protocol OGC shares with STAC, which is +; not OGC, so it sits below ``ogc`` rather than inside it (ADR 0014). + _intervals utils _querying transport @@ -38,11 +41,14 @@ layers = credentials configuration _configuration_core +; The service declaration type is a dependency-free leaf, so every layer +; above may accept one (ADR 0014). + _service exceptions ; 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/AGENTS.md b/AGENTS.md index 38bb7621..164051f4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -132,7 +132,7 @@ raise states the problem and then the action that fixes it, in that order. wrote `None` into the URL and returned an empty frame. Run the corrected call against the real service; wording review does not catch these. - Shared checks take the caller's argument name. `_validate_data_source`, - `_format_api_dates`, and `require_one_of` all accept a `name=` so the subject + `read_date_range`, and `require_one_of` all accept a `name=` so the subject of the message is the argument that was passed. A helper that hard-codes one noun reports the wrong parameter the moment a second call site reuses it. - Prefer raising over returning something empty when the library cannot distinguish diff --git a/CONTEXT.md b/CONTEXT.md index 2339dfc3..57c5d08e 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -253,15 +253,34 @@ value. Where the distinction matters — reporting what a call will use ## Boundaries -**Adapter** — A module owning one service's conventions: its URLs, parameters, -error shapes, and response quirks. Adapters may use shared code; shared code -may not reference adapters. - -**Dialect** — The per-API quirks the shared OGC code needs in order to -serve two services from one code path: which collections must be POSTed as -CQL2, which render dates date-only, which columns to coerce and sort by. An -adapter supplies one and the shared code reads it, which is how protocol code -stays free of service names. +**Adapter** — A module owning one service's conventions: its getters, public +parameters, and their translation, error shapes, and response quirks. Adapters +may use shared code; shared code may not reference adapters. An adapter +declares its service, the endpoints it calls, and their dialects as values, +and hands those to shared code (ADR 0014). + +**Service declaration** — The value an adapter declares for its service: the +name configuration resolves its settings by, and the URL a `base_url` setting +replaces. The one place that name is written outside the configuration +roster. In code, `Service`. + +**Endpoint** — One API of a service, at one URL, speaking one protocol. Water +Data is one service with four endpoints: OGC API Features, the ratings STAC +catalog, Samples, and Statistics. The adapter builds an endpoint when a call +starts, so the URL it holds is the one in effect then; it also carries the +dialect. In code, `Endpoint`. + +**Protocol** — The rules for talking to one kind of standard API, implemented +once as functions that receive an endpoint: OGC API Features, or the ISO 8601 +interval grammar OGC and STAC share. A protocol gets its own module only when +two endpoints speak it. + +**Dialect** — How one endpoint departs from its protocol's standard, as data +the protocol code reads. Each field is one extension, restriction, or renaming, +and its default is the standard, so a default dialect describes an endpoint +that follows the standard. Fields are grouped by the code that reads them; the +date group, `DateDialect`, holds the interval grammar the protocols that write +intervals share (ADR 0014, proposed). **Single-shot adapter** — An adapter whose query is always exactly one request: WQP, NLDI, StreamStats, and deprecated NWIS. Nothing divides and nothing @@ -312,9 +331,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 5cfb69e9..bc6c86f0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,9 @@ +**10/11/2026:** **Behavior change:** `ngwmn.get_water_level()` raises `ValueError` for a lone ISO 8601 duration such as `datetime="P7D"` before sending, instead of failing with the service's HTTP 400. Pass a start and an end, such as `datetime=["2024-01-01", "2024-01-08"]`. `waterdata.get_ratings()` already refused a duration; its message now names `time` and suggests that same form. The Water Data OGC getters still accept durations. + +**10/09/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: an aware value is converted to UTC, a naive one is read in local time, 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. + +**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/11/2026:** **Bug fix:** a list passed to most `waterdata` OGC filters, such as `get_daily(approval_status=["Approved", "Provisional"])`, returned no rows: the Water Data API reads a comma-joined value as one value for all but a few identifier and code filters, and rejects a numeric list with HTTP 400. Lists are now sent as a CQL2 `in` filter. A list combined with `filter=` in `get_monitoring_locations()` and `get_combined_metadata()` is no longer ignored. **Bug fix:** `filter_lang="cql-json"` failed with HTTP 400 in every `waterdata` OGC getter. The filter is now sent as the request body; one that is not a JSON object raises `ValueError`. A blank `filter=""` is now ignored rather than failing with HTTP 400. `ngwmn` is unchanged. **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. diff --git a/dataretrieval/_dates.py b/dataretrieval/_dates.py new file mode 100644 index 00000000..32b27673 --- /dev/null +++ b/dataretrieval/_dates.py @@ -0,0 +1,255 @@ +"""What a caller meant by a date argument, read once at the public boundary. + +A dependency-free leaf (ADR 0014): a getter reads its date arguments into a +:class:`DateRange` when it is called, 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 spells an open bound, and that a naive value +is local time -- is package policy, so it lives here and names no service. +""" + +from __future__ import annotations + +import re +from collections.abc import Iterable, Mapping +from dataclasses import dataclass +from datetime import date as _date +from datetime import datetime + +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-%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 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 + +# What a getter accepts as one date argument: one element, an interval string, +# or a list of one or two elements (a range whose ``None`` end is open). +DateArgument = DateLike | Iterable[DateLike] + + +@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"``). A bound is + naive exactly when the caller gave no zone, and a naive bound is local + time. + """ + + 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 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): + # A writer converts with ``astimezone``, which on a ``Timestamp`` is + # ``tz_convert`` and requires a zone, so keep 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 bound: ``None`` when it is blank (an open bound).""" + return None if dt is None or _is_blank(dt) else _to_datetime(dt, name=name) + + +def _coerce_to_list( + value: DateArgument, + name: str, +) -> list[DateLike]: + """Normalize a date argument to a list of elements, raising on a mapping.""" + if isinstance(value, str) or not isinstance(value, Iterable): + # A lone value (a ``date`` is not iterable); a non-date is rejected + # per element. + return [value] + if isinstance(value, Mapping): + raise TypeError( + f"{name} must be a date, a string, or a sequence of them, " + f"not {type(value).__name__}." + ) + return list(value) + + +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, which the services + this package reaches answer 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 read_date_range( + value: DateArgument | 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 endpoint 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..91d9e95c --- /dev/null +++ b/dataretrieval/_intervals.py @@ -0,0 +1,78 @@ +"""The ISO 8601 interval grammar OGC API Features and STAC share (ADR 0014). + +A getter reads a date argument 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 endpoint's :class:`DateDialect`. It names no service. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime +from zoneinfo import ZoneInfo + +from dataretrieval._dates import OPEN_BOUND, DateRange + + +@dataclass(frozen=True) +class DateDialect: + """How one endpoint writes dates differently from the interval standard. + + Each default is the standard, so ``DateDialect()`` describes an endpoint + that follows it. + + Attributes + ---------- + open_start : str + How the open start of a range is written: ``".."`` (``"../2024-01-01"``, + the default) or ``""`` (``"/2024-01-01"``). Both standards allow + either, but deployments refuse one or the other; each declaration + records what its service was observed to refuse. + 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 when the getter is called. + """ + + open_start: str = OPEN_BOUND + durations: bool = False + + +def _write_bound(bound: datetime | None, *, date: bool) -> str: + """One bound: ``YYYY-MM-DD``, a UTC ``...Z`` datetime, or ``..``.""" + if bound is None: + return OPEN_BOUND + if date: + return bound.strftime("%Y-%m-%d") + # A naive bound is local time. Use ``.astimezone()`` rather than a fixed + # offset so each value is resolved against the DST rules for its own date. + aware = bound if bound.tzinfo is not None else bound.astimezone() + return aware.astimezone(ZoneInfo("UTC")).strftime("%Y-%m-%dT%H:%M:%SZ") + + +def write_interval(value: DateRange, dialect: DateDialect, *, date_only: bool) -> str: + """Write a date range as the endpoint'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 endpoint's date dialect; supplies ``open_start``. + date_only : bool + Write each bound as ``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=date_only) + start = ( + dialect.open_start + if value.start is None + else _write_bound(value.start, date=date_only) + ) + return f"{start}/{_write_bound(value.end, date=date_only)}" diff --git a/dataretrieval/_service.py b/dataretrieval/_service.py new file mode 100644 index 00000000..a1b5c0ce --- /dev/null +++ b/dataretrieval/_service.py @@ -0,0 +1,55 @@ +"""The value an adapter declares for its service (ADR 0014). + +A dependency-free leaf, so configuration, execution, and protocol code can all +accept a :class:`Service` without importing an adapter. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Generic, TypeVar + + +@dataclass(frozen=True) +class Service: + """One external system this package retrieves from. + + Attributes + ---------- + name : str + The adapter's name: the key of its configuration table and its + ``Configuration`` class, and the name settings are resolved by. It must + be listed in the configuration roster. + default_url : str + The URL used when no ``base_url`` setting replaces it. + """ + + name: str + default_url: str + + +DialectT = TypeVar("DialectT") + + +@dataclass(frozen=True) +class Endpoint(Generic[DialectT]): + """One API of a service, as a call reaches it (ADR 0014). + + An adapter builds one when a call starts, so its URL is the one the + configuration in effect at that moment resolves; every chunk, and a later + resume, use the same value. + + Attributes + ---------- + service : Service + The service this API belongs to; settings resolve by its name. + url : str + The API's base URL for this call. + dialect : DialectT + How this API departs from its protocol's standard. The type is the + protocol's own dialect type. + """ + + service: Service + url: str + dialect: DialectT diff --git a/dataretrieval/configuration.py b/dataretrieval/configuration.py index 6437d789..bb4b9aee 100644 --- a/dataretrieval/configuration.py +++ b/dataretrieval/configuration.py @@ -64,7 +64,7 @@ from contextlib import contextmanager from functools import partial from pathlib import Path -from typing import TextIO, overload +from typing import TYPE_CHECKING, TextIO, overload # Explicit same-name aliases preserve the facade's public and private compatibility # symbols for runtime users and static analyzers. Ruff otherwise expands these @@ -122,6 +122,9 @@ # isort: on from dataretrieval.exceptions import ConfigurationError as ConfigurationError +if TYPE_CHECKING: + from dataretrieval._service import Service + __all__ = [ "ADAPTERS", # The package-wide configuration, and the base every adapter subclasses. @@ -658,6 +661,11 @@ def base_url(*, adapter: str | None = None, default: str | None = None) -> str | return _parse_base_url(raw, label) +def service_url(service: Service) -> str: + """*service*'s base URL: its ``base_url`` setting, or its default.""" + return base_url(adapter=service.name, default=service.default_url) + + @overload def api_version(*, adapter: str | None = ...) -> str | None: ... diff --git a/dataretrieval/ngwmn.py b/dataretrieval/ngwmn.py index 4055103b..017267aa 100644 --- a/dataretrieval/ngwmn.py +++ b/dataretrieval/ngwmn.py @@ -4,7 +4,8 @@ (``https://api.waterdata.usgs.gov/ngwmn/ogcapi``) with five collections: ``sites``, ``waterLevelObs``, ``lithologyObs``, ``constructionObs``, and ``providers``. Each getter below delegates to the shared OGC facade -(:func:`~dataretrieval.ogc.get_ogc_data`) with ``base_url=NGWMN_OGC_API_URL``. +(:func:`~dataretrieval.ogc.get_ogc_data`) with an endpoint carrying +``NGWMN_DIALECT``. Multi-value chunking, pagination, retry/resume, and result shaping therefore behave as they do for the main Water Data getters. @@ -24,6 +25,8 @@ import pandas as pd from dataretrieval import configuration as _configuration +from dataretrieval._intervals import DateDialect +from dataretrieval._service import Endpoint, Service from dataretrieval.codes.states import apply_state from dataretrieval.configuration import ( BaseConfiguration, @@ -34,9 +37,15 @@ _Retrying, ) from dataretrieval.credentials import WATERDATA_BASE_URL -from dataretrieval.ogc import OgcDialect, get_ogc_data, prepare_request_args +from dataretrieval.ogc import ( + OgcDialect, + ShapingDialect, + get_ogc_data, + prepare_request_args, +) if TYPE_CHECKING: + from dataretrieval._dates import DateArgument from dataretrieval._response_metadata import BaseMetadata __all__ = [ @@ -57,6 +66,9 @@ # separate, unversioned base. NGWMN_OGC_API_URL = f"{BASE_URL}/ngwmn/ogcapi" +#: This adapter's service (ADR 0014). +_SERVICE = Service("ngwmn", NGWMN_OGC_API_URL) + # --- state-filter shim ------------------------------------------------------- # NGWMN's collections expose different state queryables: ``sites`` filters on # the full ``state_name`` (e.g. "Wisconsin"), while ``providers`` filters on the @@ -80,22 +92,23 @@ # (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: it reads comma-joined -# GET values as lists, and its firewall refuses a text filter naming a site, so -# a list could not be folded into one. It has no 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. +# Lists stay comma-joined: NGWMN reads them as lists, and its firewall refuses a +# text filter naming a site, so a list could not be folded into one. +# ``open_start=""``: the firewall answers ``datetime=../2016-01-01`` with HTTP +# 403 and reads ``/2016-01-01`` as open (observed 2026-10-11). NGWMN_DIALECT = OgcDialect( - time_cols=frozenset({"sample_time"}), - numerical_cols=frozenset( - { - "water_depth_below_land_surface_ft", - "water_level_above_site_datum_ft", - "water_level_above_navd88_ft", - } + dates=DateDialect(open_start=""), + shaping=ShapingDialect( + time_cols=frozenset({"sample_time"}), + numerical_cols=frozenset( + { + "water_depth_below_land_surface_ft", + "water_level_above_site_datum_ft", + "water_level_above_navd88_ft", + } + ), + sort_cols=("sample_time", "monitoring_location_id"), ), - sort_cols=("sample_time", "monitoring_location_id"), ) @@ -108,18 +121,17 @@ 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, output_id=_NGWMN_OUTPUT_ID, - # A ``NgwmnConfiguration(base_url=...)`` from an enclosing block, or - # this service's own base. Read here because this is the one place the - # NGWMN base is named (ADR 0011). - base_url=_configuration.base_url(adapter="ngwmn", default=NGWMN_OGC_API_URL), + # Built when the call starts, so an enclosing + # ``NgwmnConfiguration(base_url=...)`` applies (ADR 0011, ADR 0014). + endpoint=Endpoint( + _SERVICE, _configuration.service_url(_SERVICE), NGWMN_DIALECT + ), spatial=service == "sites", - dialect=NGWMN_DIALECT, - adapter="ngwmn", ) @@ -239,7 +251,7 @@ def get_water_level( water_level_above_site_datum_ft: str | Iterable[str] | None = None, monitoring_location_vertical_datum: str | Iterable[str] | None = None, water_level_above_navd88_ft: str | Iterable[str] | None = None, - datetime: str | Iterable[str] | None = None, + datetime: DateArgument | None = None, properties: str | Iterable[str] | None = None, limit: int | None = None, convert_type: bool = True, @@ -268,7 +280,8 @@ def get_water_level( datetime : str or iterable of str, optional Temporal filter — a single instant or a two-element ``[start, end]`` range (ISO-8601 dates/datetimes); ``None`` or ``".."`` denotes an - open end. + open end. A duration such as ``"P7D"`` raises ``ValueError``, because + the service refuses one. properties : str or iterable of str, optional Subset of columns to return. ``None`` (default) returns all columns. limit : int, optional @@ -481,7 +494,7 @@ class NgwmnConfiguration( rate-limit quota, so raise it only for pulls you know are large. """ - adapter: ClassVar[str] = "ngwmn" + adapter: ClassVar[str] = _SERVICE.name _register(NgwmnConfiguration) diff --git a/dataretrieval/nldi.py b/dataretrieval/nldi.py index 33b92027..264546a9 100644 --- a/dataretrieval/nldi.py +++ b/dataretrieval/nldi.py @@ -16,6 +16,7 @@ from dataretrieval import configuration as _configuration from dataretrieval._querying import _query_with_retry +from dataretrieval._service import Service from dataretrieval._validation import ( reject_together, render_options, @@ -51,6 +52,9 @@ ) from err NLDI_API_BASE_URL = "https://api.water.usgs.gov/nldi/linked-data" + +#: This adapter's service (ADR 0014). +_SERVICE = Service("nldi", NLDI_API_BASE_URL) _AVAILABLE_DATA_SOURCES = None _CRS = "EPSG:4326" _VALID_NAVIGATION_MODES = ("UM", "DM", "UT", "DD") @@ -73,7 +77,7 @@ def _api_base() -> str: :data:`NLDI_API_BASE_URL` directly, so a ``NldiConfiguration(base_url=...)`` applies to every navigation, basin, and catalog request alike (ADR 0011). """ - return _configuration.base_url(adapter="nldi", default=NLDI_API_BASE_URL) + return _configuration.service_url(_SERVICE) def _query_nldi( @@ -83,7 +87,7 @@ def _query_nldi( # A helper function to query the NLDI API. ``query()`` already raises a typed # ``DataRetrievalError`` for any HTTP error response, so a returned response is a # success that only needs parsing. - response = _query_with_retry(url, payload=query_params, adapter="nldi") + response = _query_with_retry(url, payload=query_params, adapter=_SERVICE.name) response_data: dict[str, Any] | list[Any] = {} try: response_data = response.json() @@ -734,7 +738,7 @@ class registers itself later than the rest -- which is why environment refuse it. """ - adapter: ClassVar[str] = "nldi" + adapter: ClassVar[str] = _SERVICE.name _register(NldiConfiguration) diff --git a/dataretrieval/nwdc.py b/dataretrieval/nwdc.py index a42b8254..44e54b15 100644 --- a/dataretrieval/nwdc.py +++ b/dataretrieval/nwdc.py @@ -48,6 +48,7 @@ from dataretrieval._csv import read_code_csv from dataretrieval._querying import _raise_for_status, to_str from dataretrieval._response_metadata import BaseMetadata +from dataretrieval._service import Service from dataretrieval._validation import render_options, require_exactly_one from dataretrieval.codes.states import to_state from dataretrieval.configuration import ( @@ -72,6 +73,9 @@ ] WATERUSE_URL = "https://api.water.usgs.gov/nwaa-data/data" + +#: This adapter's service (ADR 0014). +_SERVICE = Service("nwdc", WATERUSE_URL) _WATERUSE_HOST = httpx.URL(WATERUSE_URL).host # Hosts a ``rel="next"`` cursor may name for this same service; each is # rewritten to :data:`_WATERUSE_HOST` rather than followed as given. @@ -244,22 +248,22 @@ def get_wateruse( # An ``NwdcConfiguration(base_url=...)`` from an enclosing block, or this # service's own endpoint, passed to every request and the page walk # (ADR 0011). - service_url = _configuration.base_url(adapter="nwdc", default=WATERUSE_URL) + root = _configuration.service_url(_SERVICE) # The NWDC queries one location per request, so fan a multi-value selector # out into one request per location, each handled by shared transport # pagination, and concatenate the results. - headers = default_headers(service_url) + headers = default_headers(root) requests = [ httpx.Request( "GET", - service_url, + root, params={**base_params, "location": location}, headers=headers, ) for location in _resolve_locations(state, county, huc) ] - return _fan_out(requests, headers, ssl_check, host=httpx.URL(service_url).host) + return _fan_out(requests, headers, ssl_check, host=httpx.URL(root).host) # Valid HUC code lengths (digits) → the hydrologic-unit level they query. @@ -380,7 +384,7 @@ def finalize( client_options={"verify": ssl_check}, default_concurrent=DEFAULT_CONCURRENT_REQUESTS, service="nwdc", - adapter="nwdc", + adapter=_SERVICE.name, ) @@ -488,7 +492,7 @@ class NwdcConfiguration(_Concurrent, _Redirectable, _Retrying, BaseConfiguration Cap on simultaneous sub-requests, or ``"unbounded"``. """ - adapter: ClassVar[str] = "nwdc" + adapter: ClassVar[str] = _SERVICE.name _register(NwdcConfiguration) diff --git a/dataretrieval/ogc/__init__.py b/dataretrieval/ogc/__init__.py index 1f43cb92..0e70e72b 100644 --- a/dataretrieval/ogc/__init__.py +++ b/dataretrieval/ogc/__init__.py @@ -2,7 +2,8 @@ The public facade exposes only the minimal collection-adapter seam: -- :class:`OgcDialect` — per-API request/response quirks. +- :class:`OgcDialect` — how an API departs from OGC API Features, with its + :class:`ShapingDialect` group of result-shaping fields. - :func:`prepare_request_args` — normalize caller kwargs for the engine. - :func:`get_ogc_data` — full orchestrated OGC fetch (chunking + pagination), including verbatim-CQL2 queries via its ``cql_body`` parameter. @@ -14,11 +15,12 @@ """ from dataretrieval.ogc.engine import get_ogc_data -from dataretrieval.ogc.policy import OgcDialect +from dataretrieval.ogc.policy import OgcDialect, ShapingDialect from dataretrieval.ogc.requests import prepare_request_args __all__ = [ "OgcDialect", + "ShapingDialect", "get_ogc_data", "prepare_request_args", ] diff --git a/dataretrieval/ogc/dates.py b/dataretrieval/ogc/dates.py deleted file mode 100644 index 0fe5cd9e..00000000 --- a/dataretrieval/ogc/dates.py +++ /dev/null @@ -1,211 +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 Mapping, Sequence -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-%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 = ".." - - -def _is_blank(dt: str | None) -> 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 _format_one(dt: str | None, *, 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 - matches no supported format. - """ - if dt is None or _is_blank(dt): - return _OPEN_BOUND - parsed = _parse_datetime(dt) - 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', and None for an open end of a range " - "(['2024-01-01', None])." - ) - if date: - return parsed.strftime("%Y-%m-%d") - # Naive inputs are interpreted in the system local zone (for backwards - # compatibility). Use ``.astimezone()`` rather than a fixed offset so each - # value is resolved against the DST rules for its own date — a fixed - # ``datetime.now()`` offset shifted off-season inputs by an hour. - aware = parsed if parsed.tzinfo is not None else parsed.astimezone() - return aware.astimezone(ZoneInfo("UTC")).strftime("%Y-%m-%dT%H:%M:%SZ") - - -def _coerce_to_list( - datetime_input: str | Sequence[str | None], - name: str = "date input", -) -> list[str | None]: - """Normalize datetime input to a list, raising on invalid shapes.""" - if isinstance(datetime_input, str): - return [datetime_input] - if isinstance(datetime_input, Mapping): - raise TypeError( - f"{name} must be a string or sequence of strings, " - f"not {type(datetime_input).__name__}." - ) - 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 _all_blank(items: list[str | None]) -> 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 | Sequence[str | None] | None, - date: bool = False, - *, - name: str = "date input", - single_value_hint: str = "an instant or a duration ('2020-01-01', 'P7D')", -) -> 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, List[Optional[str]], None] - A single date/datetime string or a list of one or two date/datetime - strings. Accepts 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. - - 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. - - Notes - ----- - - A single blank/NA value returns None. In a two-value range, a blank/NA - 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. - - 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 - local time zone for backwards compatibility. - """ - if datetime_input is None: - return None - - items = _coerce_to_list(datetime_input, 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 duration ("P7D", "PT36H") and pre-formatted interval ("a/b") - if len(items) == 1 and isinstance(items[0], str) and _is_passthrough(items[0]): - return items[0] - - return "/".join(_format_one(dt, date=date, name=name) for dt in items) diff --git a/dataretrieval/ogc/engine.py b/dataretrieval/ogc/engine.py index b4aa891f..cef1ed45 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 @@ -16,10 +15,8 @@ * ``output_id`` — the user-facing column the wire ``id`` is renamed to, passed explicitly (no collection map is defined here). -* ``base_url`` — the OGC API base to target. -* ``extra_id_cols`` — synthetic id columns to move to the end of a result. -* ``dialect`` — an :class:`OgcDialect` describing how lists are sent and - which collections use date-only (vs. full datetime) time arguments. +* ``endpoint`` — the API to target, its service, and its + :class:`OgcDialect` (ADR 0014). """ from __future__ import annotations @@ -37,7 +34,6 @@ import dataretrieval.ogc.chunking as chunking from dataretrieval.ogc.errors import _raise_for_non_200 from dataretrieval.ogc.policy import ( - DEFAULT_DIALECT, OgcDialect, _require_positive_int, ) @@ -47,6 +43,7 @@ from dataretrieval.ogc.requests import ( _construct_api_requests, _construct_cql_request, + _read_raw_dates, _switch_arg_id, _switch_properties_id, ) @@ -58,6 +55,7 @@ if TYPE_CHECKING: from dataretrieval._response_metadata import BaseMetadata + from dataretrieval._service import Endpoint # Set up logger for this module logger = logging.getLogger(__name__) @@ -219,13 +217,10 @@ def get_ogc_data( collection: str, output_id: str, *, - base_url: str, + endpoint: Endpoint[OgcDialect], spatial: bool, max_rows: int | None = None, - extra_id_cols: frozenset[str] | set[str] = frozenset(), - dialect: OgcDialect | None = None, cql_body: str | None = None, - adapter: str | None = None, ) -> tuple[pd.DataFrame, BaseMetadata]: """ Retrieves OGC (Open Geospatial Consortium) data as a DataFrame with metadata. @@ -249,20 +244,14 @@ def get_ogc_data( truncate the result to exactly ``max_rows``. ``None`` (default) fetches the full result. Intended for few-request previews of large, un-chunked tables (e.g. :func:`get_reference_table`). - base_url : str - OGC API base URL to target. Required: this package is API-neutral and - names no API of its own, so each adapter passes its own base (e.g. - ``waterdata.utils.OGC_API_URL``, ``ngwmn.NGWMN_OGC_API_URL``). + endpoint : Endpoint[OgcDialect] + The API to target, built by the adapter when the call starts: its URL, + the service whose settings apply, and how it departs from the standard + (ADR 0014). This package names no API of its own. spatial : bool Whether this collection's result contract includes feature geometry. The adapter supplies this semantic fact; ``skip_geometry`` and geopandas availability then select the concrete frame representation. - extra_id_cols : set or frozenset, optional - Synthetic id columns to move to the end of a result frame (see - :func:`_arrange_cols`). Defaults to an empty set. - dialect : OgcDialect, optional - Per-API request quirks (how lists are sent, date-only collections). - Defaults to a plain OGC API. cql_body : str, optional A verbatim CQL2 JSON body to POST against ``collection`` instead of building the query from ``args``. With a body, only the @@ -291,8 +280,7 @@ def get_ogc_data( if max_rows is not None: _require_positive_int(max_rows, "max_rows") - if dialect is None: - dialect = DEFAULT_DIALECT + adapter = endpoint.service.name args = args.copy() args["collection"] = collection args = _switch_arg_id(args, id_name=output_id, collection=collection) @@ -308,6 +296,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, endpoint.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 @@ -332,9 +321,7 @@ def get_ogc_data( geopd=geopd, include_geometry=include_geometry, max_rows=max_rows, - extra_id_cols=extra_id_cols, - dialect=dialect, - base_url=base_url, + endpoint=endpoint, ) if cql_body is not None: @@ -344,7 +331,7 @@ def get_ogc_data( req = _construct_cql_request( collection, cql_body, - base_url=base_url, + base_url=endpoint.url, properties=args.get("properties"), bbox=args.get("bbox"), limit=args.get("limit"), @@ -376,9 +363,7 @@ def get_ogc_data( # rebuilds them through these same bound callables, so the values the # call was created with apply to every chunk — even a resume invoked long # after this function returned — without any ambient state to snapshot. - build_request = functools.partial( - _construct_api_requests, base_url=base_url, dialect=dialect - ) + build_request = functools.partial(_construct_api_requests, endpoint=endpoint) fetch = functools.partial( _fetch_once, build_request=build_request, 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 e40fa6a8..7fed18c7 100644 --- a/dataretrieval/ogc/policy.py +++ b/dataretrieval/ogc/policy.py @@ -1,13 +1,14 @@ -"""Low-level OGC policy: the dialect type and control validation. +"""Low-level OGC policy: the dialect types and control validation. -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. +This module is the one definition of :class:`OgcDialect` (how an API departs +from OGC API Features, read by the request builder and the result finalizer), +its :class:`ShapingDialect` group, and OGC control validation. +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 -direct every generic OGC caller to one API. +It names no API: which one an OGC call targets is the *adapter's* policy, +supplied per call as an endpoint (ADR 0014). A default here would direct every +generic OGC caller to one API. It must not import engine, shaping, or any collection adapter. """ @@ -17,6 +18,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 @@ -37,39 +40,64 @@ def _require_positive_int( @dataclass(frozen=True) -class OgcDialect: - """Per-API differences the generic request builder must handle. +class ShapingDialect: + """How an API's results are shaped into a frame: the fields the result + finalizer reads together. Each default leaves the result as sent. Attributes ---------- - lists_as_cql2 : bool - 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. 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 - supplies its own. + Result columns to coerce to datetime when ``convert_type`` is set. numerical_cols : frozenset[str] Result columns to coerce to numeric when ``convert_type`` is set. sort_cols : tuple[str, ...] 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. + extra_id_cols : frozenset[str] + Synthetic per-record id columns, with no meaning to a user, moved to + the end of a result frame when ``properties`` was not given. """ - lists_as_cql2: bool = False - date_only_services: 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) + time_cols: frozenset[str] = frozenset() + numerical_cols: frozenset[str] = frozenset() + sort_cols: tuple[str, ...] = () + extra_id_cols: frozenset[str] = frozenset() + + +@dataclass(frozen=True) +class OgcDialect: + """How one OGC API departs from OGC API Features (ADR 0014). + + Each default is the standard, so ``OgcDialect()`` describes an API that + follows it. + Attributes + ---------- + lists_as_cql2 : bool + 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_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 and a naive bound + converted to UTC could move to the neighboring day. + timed_params : frozenset[str] + Parameters written as a UTC datetime even in a date-only collection, + because they filter a timestamp. + dates : DateDialect + How the API's date values are read and written: the interval grammar + it shares with STAC. + shaping : ShapingDialect + How the API's results are shaped into a frame. + """ -# Default dialect: a plain OGC API, with lists comma-joined in the URL and every -# time argument rendered as a full UTC datetime. -DEFAULT_DIALECT = OgcDialect() + lists_as_cql2: bool = False + date_params: frozenset[str] = frozenset({"datetime"}) + date_only_collections: frozenset[str] = frozenset() + timed_params: frozenset[str] = frozenset() + dates: DateDialect = field(default_factory=DateDialect) + shaping: ShapingDialect = field(default_factory=ShapingDialect) diff --git a/dataretrieval/ogc/requests.py b/dataretrieval/ogc/requests.py index 674b41b7..60305ddc 100644 --- a/dataretrieval/ogc/requests.py +++ b/dataretrieval/ogc/requests.py @@ -1,7 +1,7 @@ """OGC argument normalization and HTTP request construction. -The API to target and its quirks are explicit parameters (``base_url``, -``dialect``) -- construction takes everything it needs as parameters. +The API to target and its quirks are one explicit parameter, the endpoint +(ADR 0014) -- construction takes everything it needs as parameters. Queryables and schema execution are in :mod:`dataretrieval.ogc.schema` and are not re-exported here, so this module does not depend on the part of OGC that executes HTTP (ADR 0007). @@ -12,15 +12,19 @@ import json import re from collections.abc import Iterable, Mapping -from typing import Any +from typing import TYPE_CHECKING, Any 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 +if TYPE_CHECKING: + from dataretrieval._service import Endpoint + from dataretrieval.ogc.policy import OgcDialect + # --------------------------------------------------------------------------- # Monitoring location ID validation # --------------------------------------------------------------------------- @@ -190,28 +194,22 @@ def _construct_api_requests( limit: int | None = None, skip_geometry: bool | None = None, *, - base_url: str, - dialect: OgcDialect | None = None, + endpoint: Endpoint[OgcDialect], **kwargs: Any, ) -> httpx.Request: """Construct an HTTP request object for the specified OGC API collection. - ``base_url`` is required: this package is API-neutral and names no API of - its own, so the adapter naming the collection also names the API it targets. - ``dialect`` defaults to a plain OGC API with no per-collection quirks. + ``endpoint`` is required: this package names no API of its own, so the + adapter naming the collection also names the API it targets. """ - 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" - ), - ) + service_url = _items_url(collection, endpoint.url) + dialect = endpoint.dialect + for key in dialect.date_params & kwargs.keys(): + date_only = ( + collection in dialect.date_only_collections + and key not in dialect.timed_params + ) + kwargs[key] = write_interval(kwargs[key], dialect.dates, date_only=date_only) json_filter = _pop_json_filter(kwargs) params, post_params = _partition_request_params( kwargs, use_cql2=dialect.lists_as_cql2 @@ -256,8 +254,7 @@ def _construct_cql_request( ) -> httpx.Request: """Build a POST/CQL2 request from a verbatim CQL2 body. - ``base_url`` is required for the same reason as in - :func:`_construct_api_requests`. + ``base_url`` is required: this package names no API of its own. """ service_url = _items_url(collection, base_url) params = _ogc_query_params( @@ -275,16 +272,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: @@ -309,7 +305,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) @@ -341,16 +337,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 by the dialect of the endpoint + 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, so a verbatim + ``query`` that skipped :func:`prepare_request_args` is read too. + + 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 endpoint's dialect " + f"has no date parameter {key!r}. Read the arguments with the " + f"dialect of the endpoint 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) @@ -366,18 +396,22 @@ 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`` -- the one the call's endpoint carries -- 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"} @@ -386,7 +420,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/ogc/shaping.py b/dataretrieval/ogc/shaping.py index 8e874606..0a5511d4 100644 --- a/dataretrieval/ogc/shaping.py +++ b/dataretrieval/ogc/shaping.py @@ -13,13 +13,16 @@ import logging import re import warnings -from typing import Any +from typing import TYPE_CHECKING, Any import httpx import pandas as pd from dataretrieval._response_metadata import BaseMetadata -from dataretrieval.ogc.policy import DEFAULT_DIALECT, OgcDialect + +if TYPE_CHECKING: + from dataretrieval._service import Endpoint + from dataretrieval.ogc.policy import OgcDialect, ShapingDialect try: import geopandas as gpd @@ -220,7 +223,7 @@ def _arrange_cols( df: pd.DataFrame, properties: list[str] | None, output_id: str, - extra_id_cols: frozenset[str] | set[str] = frozenset(), + extra_id_cols: frozenset[str] = frozenset(), ) -> pd.DataFrame: """ Rearranges and renames DataFrame columns per ``properties`` and the output id. @@ -271,7 +274,7 @@ def _arrange_cols( return df.loc[:, [col for col in local_properties if col in df.columns]] -def _type_cols(df: pd.DataFrame, dialect: OgcDialect) -> pd.DataFrame: +def _type_cols(df: pd.DataFrame, dialect: ShapingDialect) -> pd.DataFrame: """ Casts columns into appropriate types per the API ``dialect``. @@ -279,7 +282,7 @@ def _type_cols(df: pd.DataFrame, dialect: OgcDialect) -> pd.DataFrame: ---------- df : pd.DataFrame The input DataFrame. - dialect : OgcDialect + dialect : ShapingDialect Supplies ``time_cols`` / ``numerical_cols`` — which columns to coerce to datetime/numeric. The engine itself holds no API-specific column list. @@ -333,7 +336,7 @@ def _warn_unparsed(raw: pd.Series, parsed: pd.Series, col: str, kind: str) -> No ) -def _sort_rows(df: pd.DataFrame, dialect: OgcDialect) -> pd.DataFrame: +def _sort_rows(df: pd.DataFrame, dialect: ShapingDialect) -> pd.DataFrame: """ Sorts rows by the API ``dialect``'s ``sort_cols`` (in priority order). @@ -347,7 +350,7 @@ def _sort_rows(df: pd.DataFrame, dialect: OgcDialect) -> pd.DataFrame: ---------- df : pd.DataFrame The input DataFrame. - dialect : OgcDialect + dialect : ShapingDialect Supplies ``sort_cols``. Returns @@ -398,9 +401,7 @@ def _finalize_ogc( geopd: bool, include_geometry: bool, max_rows: int | None = None, - extra_id_cols: frozenset[str] | set[str] = frozenset(), - dialect: OgcDialect | None = None, - base_url: str, + endpoint: Endpoint[OgcDialect], ) -> tuple[pd.DataFrame, BaseMetadata]: """Shape a combined OGC result into the user-facing ``(df, md)``. @@ -418,16 +419,15 @@ def _finalize_ogc( rather than only per-chunk, so a chunked call's total is bounded to exactly ``max_rows`` and a resumed call applies the cap too. The per-page ``row_cap`` bound in the engine is only an early-stop download - bound. ``base_url`` is required and captured with the finalizer so resumed - calls query the same API's schema when their combined result is empty. + bound. ``endpoint`` is captured with the finalizer so resumed calls query + the same API's schema when their combined result is empty. """ - if dialect is None: - dialect = DEFAULT_DIALECT + dialect = endpoint.dialect.shaping frame = _deal_with_empty( frame, properties, collection, - base_url=base_url, + base_url=endpoint.url, geopd=geopd, include_geometry=include_geometry, ) @@ -445,7 +445,7 @@ def _finalize_ogc( frame = frame.rename(columns=renames) if convert_type: frame = _type_cols(frame, dialect) - frame = _arrange_cols(frame, properties, output_id, extra_id_cols) + frame = _arrange_cols(frame, properties, output_id, dialect.extra_id_cols) frame = _sort_rows(frame, dialect) if max_rows is not None: frame = frame.head(max_rows) diff --git a/dataretrieval/streamstats.py b/dataretrieval/streamstats.py index f8c14e2c..a99944a4 100644 --- a/dataretrieval/streamstats.py +++ b/dataretrieval/streamstats.py @@ -14,6 +14,7 @@ from dataretrieval import configuration as _configuration from dataretrieval._querying import _get_with_retry +from dataretrieval._service import Service from dataretrieval.configuration import ( BaseConfiguration, _Redirectable, @@ -32,6 +33,9 @@ STREAMSTATS_URL = "https://streamstats.usgs.gov/streamstatsservices" +#: This adapter's service (ADR 0014). +_SERVICE = Service("streamstats", STREAMSTATS_URL) + def _service_base() -> str: """The StreamStats base this call targets: a block's redirect, or its own. @@ -40,7 +44,7 @@ def _service_base() -> str: ``StreamstatsConfiguration(base_url=...)`` redirects the whole service rather than one endpoint (ADR 0011). """ - return _configuration.base_url(adapter="streamstats", default=STREAMSTATS_URL) + return _configuration.service_url(_SERVICE) def download_workspace(workspaceID: str, format: str = "") -> httpx.Response: @@ -65,7 +69,7 @@ def download_workspace(workspaceID: str, format: str = "") -> httpx.Response: payload = {"workspaceID": workspaceID, "format": format} url = f"{_service_base()}/download" - r = _get_with_retry(url, params=payload, adapter="streamstats", **HTTPX_DEFAULTS) + r = _get_with_retry(url, params=payload, adapter=_SERVICE.name, **HTTPX_DEFAULTS) return r # data = r.raw.read() @@ -168,7 +172,7 @@ def get_watershed( } url = f"{_service_base()}/watershed.geojson" - r = _get_with_retry(url, params=payload, adapter="streamstats", **HTTPX_DEFAULTS) + r = _get_with_retry(url, params=payload, adapter=_SERVICE.name, **HTTPX_DEFAULTS) if format == "geojson": return r @@ -264,7 +268,7 @@ class StreamstatsConfiguration(_Redirectable, _Retrying, BaseConfiguration): the file and the environment refuse it. """ - adapter: ClassVar[str] = "streamstats" + adapter: ClassVar[str] = _SERVICE.name _register(StreamstatsConfiguration) diff --git a/dataretrieval/waterdata/configuration.py b/dataretrieval/waterdata/configuration.py index b82bff1d..da54f601 100644 --- a/dataretrieval/waterdata/configuration.py +++ b/dataretrieval/waterdata/configuration.py @@ -20,6 +20,7 @@ _Retrying, _Versioned, ) +from dataretrieval.waterdata.endpoints import SERVICE __all__ = ["WaterdataConfiguration"] @@ -71,7 +72,7 @@ class WaterdataConfiguration( # executed concurrently -- both fan-out settings. Each group declares the # setting itself once, in :mod:`dataretrieval.configuration`, which also # defines its grammar and its coercion. - adapter: ClassVar[str] = "waterdata" + adapter: ClassVar[str] = SERVICE.name _register(WaterdataConfiguration) diff --git a/dataretrieval/waterdata/endpoints.py b/dataretrieval/waterdata/endpoints.py index 978a009f..e3384b62 100644 --- a/dataretrieval/waterdata/endpoints.py +++ b/dataretrieval/waterdata/endpoints.py @@ -9,8 +9,14 @@ from __future__ import annotations from dataretrieval import configuration as _configuration +from dataretrieval._service import Service from dataretrieval.credentials import WATERDATA_BASE_URL +#: The Water Data service (ADR 0014). Its OGC, Samples, Statistics, and STAC +#: APIs are endpoints below this one root, resolved by the functions below. +#: Public within the package because each of those families reads it. +SERVICE = Service("waterdata", WATERDATA_BASE_URL) + #: The version of the Water Data OGC API this release requests by default and #: shapes responses for. OGC_API_VERSION = "v1" @@ -34,8 +40,7 @@ def _endpoint(path: str) -> str: """Return *path* beneath the effective Water Data root for this call.""" - root = _configuration.base_url(adapter="waterdata", default=WATERDATA_BASE_URL) - return f"{root}{path}" + return f"{_configuration.service_url(SERVICE)}{path}" def ogc_api_url(*, api_version: str | None = None) -> str: @@ -52,7 +57,7 @@ def ogc_api_url(*, api_version: str | None = None) -> str: """ if api_version is None: api_version = _configuration.api_version( - adapter="waterdata", default=OGC_API_VERSION + adapter=SERVICE.name, default=OGC_API_VERSION ) return _endpoint(f"{_OGC_API_PATH}/{api_version}") diff --git a/dataretrieval/waterdata/measurements.py b/dataretrieval/waterdata/measurements.py index 51b184b0..906b0dea 100644 --- a/dataretrieval/waterdata/measurements.py +++ b/dataretrieval/waterdata/measurements.py @@ -18,6 +18,7 @@ ) if TYPE_CHECKING: + from dataretrieval._dates import DateArgument from dataretrieval._response_metadata import BaseMetadata from dataretrieval.ogc.filters import FILTER_LANG @@ -32,12 +33,12 @@ def get_field_measurements( unit_of_measure: str | Iterable[str] | None = None, qualifier: str | Iterable[str] | None = None, value: str | Iterable[str] | None = None, - last_modified: str | Iterable[str] | None = None, + last_modified: DateArgument | None = None, observing_procedure: str | Iterable[str] | None = None, vertical_datum: str | Iterable[str] | None = None, measuring_agency: str | Iterable[str] | None = None, skip_geometry: bool | None = None, - time: str | Iterable[str] | None = None, + time: DateArgument | None = None, bbox: list[float] | None = None, limit: int | None = None, filter: str | None = None, @@ -243,8 +244,8 @@ def get_peaks( parameter_code: str | Iterable[str] | None = None, time_series_id: str | Iterable[str] | None = None, unit_of_measure: str | Iterable[str] | None = None, - time: str | Iterable[str] | None = None, - last_modified: str | Iterable[str] | None = None, + time: DateArgument | None = None, + last_modified: DateArgument | None = None, water_year: int | list[int] | None = None, year: int | list[int] | None = None, month: int | list[int] | None = None, @@ -382,7 +383,7 @@ def get_channel( monitoring_location_id: str | Iterable[str] | None = None, field_visit_id: str | Iterable[str] | None = None, measurement_number: str | Iterable[str] | None = None, - time: str | Iterable[str] | None = None, + time: DateArgument | None = None, channel_name: str | Iterable[str] | None = None, channel_flow: str | Iterable[str] | None = None, channel_flow_unit: str | Iterable[str] | None = None, @@ -401,7 +402,7 @@ def get_channel( vertical_velocity_description: str | Iterable[str] | None = None, longitudinal_velocity_description: str | Iterable[str] | None = None, measurement_type: str | Iterable[str] | None = None, - last_modified: str | Iterable[str] | None = None, + last_modified: DateArgument | None = None, channel_measurement_type: str | Iterable[str] | None = None, properties: str | Iterable[str] | None = None, skip_geometry: bool | None = None, diff --git a/dataretrieval/waterdata/metadata.py b/dataretrieval/waterdata/metadata.py index 3f2d14a7..92c1b463 100644 --- a/dataretrieval/waterdata/metadata.py +++ b/dataretrieval/waterdata/metadata.py @@ -21,6 +21,7 @@ ) if TYPE_CHECKING: + from dataretrieval._dates import DateArgument from dataretrieval._response_metadata import BaseMetadata from dataretrieval.ogc.filters import FILTER_LANG @@ -400,11 +401,11 @@ def get_time_series_metadata( hydrologic_unit_code: str | Iterable[str] | None = None, state: str | Iterable[str] | None = None, state_name: str | Iterable[str] | None = None, - last_modified: str | Iterable[str] | None = None, - begin: str | Iterable[str] | None = None, - end: str | Iterable[str] | None = None, - begin_utc: str | Iterable[str] | None = None, - end_utc: str | Iterable[str] | None = None, + last_modified: DateArgument | None = None, + begin: DateArgument | None = None, + end: DateArgument | None = None, + begin_utc: DateArgument | None = None, + end_utc: DateArgument | None = None, unit_of_measure: str | Iterable[str] | None = None, computation_period_identifier: str | Iterable[str] | None = None, computation_identifier: str | Iterable[str] | None = None, @@ -674,9 +675,9 @@ def get_combined_metadata( primary: str | Iterable[str] | None = None, parent_time_series_id: str | Iterable[str] | None = None, web_description: str | Iterable[str] | None = None, - last_modified: str | Iterable[str] | None = None, - begin: str | Iterable[str] | None = None, - end: str | Iterable[str] | None = None, + last_modified: DateArgument | None = None, + begin: DateArgument | None = None, + end: DateArgument | None = None, agency_code: str | Iterable[str] | None = None, agency_name: str | Iterable[str] | None = None, monitoring_location_number: str | Iterable[str] | None = None, @@ -927,9 +928,9 @@ def get_field_measurements_metadata( parameter_code: str | Iterable[str] | None = None, parameter_name: str | Iterable[str] | None = None, parameter_description: str | Iterable[str] | None = None, - begin: str | Iterable[str] | None = None, - end: str | Iterable[str] | None = None, - last_modified: str | Iterable[str] | None = None, + begin: DateArgument | None = None, + end: DateArgument | None = None, + last_modified: DateArgument | None = None, properties: str | Iterable[str] | None = None, skip_geometry: bool | None = None, bbox: list[float] | None = None, diff --git a/dataretrieval/waterdata/ratings.py b/dataretrieval/waterdata/ratings.py index f1e4ab8b..2e88e709 100644 --- a/dataretrieval/waterdata/ratings.py +++ b/dataretrieval/waterdata/ratings.py @@ -18,9 +18,11 @@ import httpx import pandas as pd +from dataretrieval._dates import DateArgument, read_date_range +from dataretrieval._intervals import DateDialect, write_interval +from dataretrieval._service import Endpoint from dataretrieval._validation import render_options from dataretrieval.exceptions import DataRetrievalError, SkippedRatingWarning -from dataretrieval.ogc.dates import _DURATION_RE, _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 @@ -32,11 +34,18 @@ from dataretrieval.transport.links import resolve_next_url from dataretrieval.transport.pagination import run_paginated from dataretrieval.transport.retry import RetryPolicy -from dataretrieval.waterdata.endpoints import ratings_catalog_url +from dataretrieval.waterdata.endpoints import SERVICE, ratings_catalog_url __all__ = ["get_ratings"] +# The STAC search's departures from the interval standard (ADR 0014). Its +# firewall answers ``datetime=../2026-01-01T00:00:00Z`` with HTTP 403, and it +# reads an empty start (``/2026-01-01T00:00:00Z``) as open; it refuses a lone +# duration with HTTP 400, as the standard does (observed 2026-10-11). +STAC_DATES = DateDialect(open_start="") + + RATING_FILE_TYPE = Literal["exsa", "base", "corr"] _VALID_FILE_TYPES = get_args(RATING_FILE_TYPE) @@ -45,7 +54,7 @@ def get_ratings( monitoring_location_id: str | Iterable[str] | None = None, file_type: RATING_FILE_TYPE | list[RATING_FILE_TYPE] = "exsa", file_path: str | None = None, - time: str | list[str] | None = None, + time: DateArgument | None = None, bbox: list[float] | None = None, limit: int = 10000, download_and_parse: bool = True, @@ -82,7 +91,7 @@ def get_ratings( persisting the bytes to disk; ``df.attrs["url"]`` still records where each rating came from. time : string or list of strings, optional - STAC ``datetime`` filter (passed through verbatim under that name) + STAC ``datetime`` filter, sent as a UTC datetime or interval — a single date / datetime, an interval (``"start/end"``, optionally half-bounded with ``..``), or a list of two values with ``None`` for an open end (``["2026-04-29", None]``). A value @@ -176,12 +185,15 @@ 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) + # Built when the call starts, so an enclosing ``configure(base_url=...)`` + # applies to the search (ADR 0014). + endpoint = Endpoint(SERVICE, ratings_catalog_url(), STAC_DATES) + time_range = read_date_range( + time, name="time", durations=endpoint.dialect.durations + ) time_str = ( - _format_api_dates( - time, name="time", single_value_hint="an instant ('2020-01-01')" - ) - if time is not None + write_interval(time_range, endpoint.dialect, date_only=False) + if time_range is not None else None ) @@ -189,7 +201,7 @@ def get_ratings( server_file_type = file_types[0] if len(file_types) == 1 else None filter_str = _build_filter(monitoring_location_id, server_file_type) - features = _search(filter_str, time_str, bbox, limit, ssl_check) + features = _search(endpoint.url, filter_str, time_str, bbox, limit, ssl_check) if not download_and_parse: return features @@ -217,17 +229,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 - if any(_DURATION_RE.match(str(v)) for v in _as_list(time)): - 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]]: @@ -262,6 +263,7 @@ def _build_filter( def _search( + catalog_url: str, filter_str: str | None, time_str: str | None, bbox: list[float] | None, @@ -286,7 +288,7 @@ def _search( if bbox is not None: query_params["bbox"] = ",".join(map(str, bbox)) - url = f"{ratings_catalog_url()}/search" + url = f"{catalog_url}/search" req = httpx.Request("GET", url, params=query_params, headers=_default_headers(url)) def parse_response(resp: httpx.Response) -> tuple[pd.DataFrame, str | None]: @@ -318,7 +320,7 @@ async def follow_up(cursor: str, sess: httpx.AsyncClient) -> httpx.Response: raise_for_status=_raise_for_non_200, client_options={"verify": ssl_check}, service="ratings", - adapter="waterdata", + adapter=SERVICE.name, ) # Every page frame is built with a ``feature`` column, and the combine # helpers preserve it, so the empty case needs no special branch. @@ -443,12 +445,12 @@ async def fetch(feature: dict[str, Any]) -> tuple[pd.DataFrame, httpx.Response]: FanOut( features, fetch, - RetryPolicy.from_configuration(adapter="waterdata"), + RetryPolicy.from_configuration(adapter=SERVICE.name), client_options={"verify": ssl_check}, # No single URL expresses "all of these assets" -- the aggregate # reports the first, matching what a single-feature call would show. canonical_url=_asset_href(features[0]), service="ratings", - adapter="waterdata", + adapter=SERVICE.name, ).resume() return out diff --git a/dataretrieval/waterdata/samples.py b/dataretrieval/waterdata/samples.py index 7e70f6a2..4048fa15 100644 --- a/dataretrieval/waterdata/samples.py +++ b/dataretrieval/waterdata/samples.py @@ -41,7 +41,7 @@ ) from dataretrieval.waterdata.utils import ( _accept_legacy_kwargs, - _get_args, + _get_query_args, ) logger = logging.getLogger(__name__) @@ -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_query_args(locals(), exclude={"ssl_check", "profile"}) 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/stats.py b/dataretrieval/waterdata/stats.py index 0087641d..be08b00f 100644 --- a/dataretrieval/waterdata/stats.py +++ b/dataretrieval/waterdata/stats.py @@ -27,7 +27,7 @@ ) from dataretrieval.transport.http import default_headers from dataretrieval.transport.pagination import run_paginated -from dataretrieval.waterdata.endpoints import statistics_api_url +from dataretrieval.waterdata.endpoints import SERVICE, statistics_api_url __all__ = ["get_data"] @@ -282,7 +282,7 @@ async def follow_up(cursor: str, sess: httpx.AsyncClient) -> httpx.Response: raise_for_status=_raise_for_non_200, client=client, service=service, - adapter="waterdata", + adapter=SERVICE.name, ) if expand_percentiles: diff --git a/dataretrieval/waterdata/time_series.py b/dataretrieval/waterdata/time_series.py index 96b2ff9f..c5282b9d 100644 --- a/dataretrieval/waterdata/time_series.py +++ b/dataretrieval/waterdata/time_series.py @@ -19,11 +19,13 @@ from dataretrieval.waterdata import stats from dataretrieval.waterdata.utils import ( _get_args, + _get_query_args, _with_state, get_ogc_data, ) if TYPE_CHECKING: + from dataretrieval._dates import DateArgument from dataretrieval._response_metadata import BaseMetadata from dataretrieval.ogc.filters import FILTER_LANG @@ -39,9 +41,9 @@ def get_daily( unit_of_measure: str | Iterable[str] | None = None, qualifier: str | Iterable[str] | None = None, value: str | Iterable[str] | None = None, - last_modified: str | Iterable[str] | None = None, + last_modified: DateArgument | None = None, skip_geometry: bool | None = None, - time: str | Iterable[str] | None = None, + time: DateArgument | None = None, bbox: list[float] | None = None, limit: int | None = None, filter: str | None = None, @@ -275,8 +277,8 @@ def get_continuous( qualifier: str | Iterable[str] | None = None, method_category: str | Iterable[str] | None = None, value: str | Iterable[str] | None = None, - last_modified: str | Iterable[str] | None = None, - time: str | Iterable[str] | None = None, + last_modified: DateArgument | None = None, + time: DateArgument | None = None, limit: int | None = None, filter: str | None = None, filter_lang: FILTER_LANG | None = None, @@ -486,9 +488,9 @@ def get_latest_continuous( unit_of_measure: str | Iterable[str] | None = None, qualifier: str | Iterable[str] | None = None, value: str | Iterable[str] | None = None, - last_modified: str | Iterable[str] | None = None, + last_modified: DateArgument | None = None, skip_geometry: bool | None = None, - time: str | Iterable[str] | None = None, + time: DateArgument | None = None, bbox: list[float] | None = None, limit: int | None = None, filter: str | None = None, @@ -703,9 +705,9 @@ def get_latest_daily( unit_of_measure: str | Iterable[str] | None = None, qualifier: str | Iterable[str] | None = None, value: str | Iterable[str] | None = None, - last_modified: str | Iterable[str] | None = None, + last_modified: DateArgument | None = None, skip_geometry: bool | None = None, - time: str | Iterable[str] | None = None, + time: DateArgument | None = None, bbox: list[float] | None = None, limit: int | None = None, filter: str | None = None, @@ -1041,7 +1043,7 @@ def get_stats_por( ... ) """ # Build argument dictionary, omitting None values - params = _get_args( + params = _get_query_args( _with_state(locals(), to="fips_us", into="state_code"), exclude={"expand_percentiles"}, ) @@ -1188,7 +1190,7 @@ def get_stats_date_range( ... ) """ # Build argument dictionary, omitting None values - params = _get_args( + params = _get_query_args( _with_state(locals(), to="fips_us", into="state_code"), exclude={"expand_percentiles"}, ) diff --git a/dataretrieval/waterdata/utils.py b/dataretrieval/waterdata/utils.py index 3fe6b829..eb40f3ab 100644 --- a/dataretrieval/waterdata/utils.py +++ b/dataretrieval/waterdata/utils.py @@ -23,9 +23,11 @@ import pandas as pd from dataretrieval._deprecation import warn_deprecated +from dataretrieval._intervals import DateDialect +from dataretrieval._service import Endpoint from dataretrieval.codes.states import apply_state from dataretrieval.credentials import refuse_credential_keywords -from dataretrieval.ogc import OgcDialect, prepare_request_args +from dataretrieval.ogc import OgcDialect, ShapingDialect, prepare_request_args from dataretrieval.ogc import get_ogc_data as _facade_get_ogc_data # Default endpoint constants remain at their documented compatibility paths. @@ -34,6 +36,7 @@ _DEFAULT_BASE_URL, _DEFAULT_OGC_API_URL, _DEFAULT_SAMPLES_URL, + SERVICE, ogc_api_url, ) @@ -73,40 +76,53 @@ - {"monitoring_location_id", "time_series_id"} ) -# The Water Data API dialect: multi-value filters are CQL2 predicates, because -# the API reads a comma-joined GET value as one value for all but a few -# identifiers and codes; ``daily`` renders its time arguments date-only -# (``YYYY-MM-DD``), and the ``time_cols``/``numerical_cols``/``sort_cols`` are -# the Water-Data column vocabulary used to coerce datetime/numeric columns and -# to sort results. +# ``lists_as_cql2``: the API reads a comma-joined GET value as one value for all +# but a few identifiers and codes. Dates keep ``..`` for an open start (an empty +# one is HTTP 400) and allow a lone duration (both observed 2026-10-11). # ``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. WATERDATA_DIALECT = OgcDialect( lists_as_cql2=True, - date_only_services=frozenset({"daily"}), - time_cols=frozenset( + date_params=frozenset( { + "datetime", + "last_modified", "begin", "begin_utc", "end", "end_utc", - "last_modified", "time", } ), - numerical_cols=frozenset( - { - "altitude", - "altitude_accuracy", - "contributing_drainage_area", - "drainage_area", - "hole_constructed_depth", - "value", - "well_constructed_depth", - } + date_only_collections=frozenset({"daily"}), + timed_params=frozenset({"last_modified"}), + dates=DateDialect(durations=True), + shaping=ShapingDialect( + time_cols=frozenset( + { + "begin", + "begin_utc", + "end", + "end_utc", + "last_modified", + "time", + } + ), + numerical_cols=frozenset( + { + "altitude", + "altitude_accuracy", + "contributing_drainage_area", + "drainage_area", + "hole_constructed_depth", + "value", + "well_constructed_depth", + } + ), + sort_cols=("time", "monitoring_location_id"), + extra_id_cols=_EXTRA_ID_COLS, ), - sort_cols=("time", "monitoring_location_id"), ) # The Water-Data-specific extras in addition to the engine's own no-normalize set @@ -172,7 +188,23 @@ def _flatten_queryables(local_vars: dict[str, Any]) -> dict[str, Any]: def _get_args( local_vars: dict[str, Any], exclude: set[str] | None = None ) -> dict[str, Any]: - """Water-Data wrapper over :func:`~dataretrieval.ogc.prepare_request_args`. + """Request arguments for a Water Data OGC getter, its date parameters read + by :data:`WATERDATA_DIALECT`, the dialect its endpoint carries. + + See :func:`_get_query_args` for what else is done. + """ + return _get_query_args(local_vars, exclude, dialect=WATERDATA_DIALECT) + + +def _get_query_args( + local_vars: dict[str, Any], + exclude: set[str] | None = None, + *, + dialect: OgcDialect | None = None, +) -> dict[str, Any]: + """Request arguments for a Water Data getter: the Samples and Statistics + getters call this directly, since their APIs are not OGC and read no date + range; the OGC getters call :func:`_get_args`. Adds the Water Data API's extra no-normalize params (numeric params such as ``water_year``, ``thresholds``, ``boundingBox``) so they keep their @@ -181,7 +213,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. @@ -270,19 +305,14 @@ def get_ogc_data( collection, output_id, max_rows=max_rows, - # The endpoint is resolved from the active ContextVar at request time; - # the documented ``OGC_API_URL`` constant remains the default-value - # compatibility path rather than a production request destination. - base_url=ogc_api_url(api_version=api_version), + # Built when the call starts, from the configuration in effect; the + # documented ``OGC_API_URL`` constant remains a default-value + # compatibility path, not a request destination (ADR 0014). + endpoint=Endpoint( + SERVICE, ogc_api_url(api_version=api_version), WATERDATA_DIALECT + ), spatial=spatial, - extra_id_cols=_EXTRA_ID_COLS, - dialect=WATERDATA_DIALECT, cql_body=cql_body, - # Which settings table these calls read. Declared here, in the one - # wrapper every Water Data getter goes through, rather than derived - # from ``base_url``: NGWMN is served from the same host, so a URL - # does not distinguish the two adapters (ADR 0010). - adapter="waterdata", ) @@ -357,7 +387,7 @@ def wrapper(*args: Any, **kwargs: Any) -> _R: "OGC_API_URL", "SAMPLES_URL", "WATERDATA_DIALECT", - "_EXTRA_ID_COLS", + "_get_query_args", "_NO_NORMALIZE_PARAMS", "_OUTPUT_ID_BY_COLLECTION", "_accept_legacy_kwargs", diff --git a/dataretrieval/wqp.py b/dataretrieval/wqp.py index fedbba40..917f51dd 100644 --- a/dataretrieval/wqp.py +++ b/dataretrieval/wqp.py @@ -17,6 +17,7 @@ from dataretrieval import configuration as _configuration from dataretrieval._csv import read_code_csv from dataretrieval._response_metadata import BaseMetadata +from dataretrieval._service import Service from dataretrieval._validation import require_one_of from dataretrieval.configuration import ( BaseConfiguration, @@ -57,6 +58,9 @@ #: this is only the part they share, and the part a redirect replaces. _WQP_BASE_URL = "https://www.waterqualitydata.us" +#: This adapter's service (ADR 0014). +_SERVICE = Service("wqp", _WQP_BASE_URL) + result_profiles_wqx3 = ["basicPhysChem", "fullPhysChem", "narrow"] result_profiles_legacy = ["biological", "narrowResult", "resultPhysChem"] activity_profiles_legacy = ["activity", "activityAll"] @@ -150,7 +154,11 @@ def _query_wqp( delimiter = None response = _query_with_retry( - url, payload=kwargs, delimiter=delimiter, ssl_check=ssl_check, adapter="wqp" + url, + payload=kwargs, + delimiter=delimiter, + ssl_check=ssl_check, + adapter=_SERVICE.name, ) df = read_code_csv(response.text) # Only get_results documents the appended DateTime columns and the @@ -652,7 +660,7 @@ def _service_base() -> str: different paths, so a ``WqpConfiguration(base_url=...)`` names that root and both use it (ADR 0011). """ - return _configuration.base_url(adapter="wqp", default=_WQP_BASE_URL) + return _configuration.service_url(_SERVICE) def wqp_url(service: str) -> str: @@ -832,7 +840,7 @@ class WqpConfiguration(_Redirectable, _Retrying, BaseConfiguration): the environment refuse it. """ - adapter: ClassVar[str] = "wqp" + adapter: ClassVar[str] = _SERVICE.name _register(WqpConfiguration) 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..e28cbe0c --- /dev/null +++ b/docs/source/architecture/decisions/0014-services-endpoints-dialects.rst @@ -0,0 +1,306 @@ +ADR 0014: Declare services, endpoints, and dialects as values +============================================================= + +Status +------ + +Proposed. This record is revised as its implementation lands, one pull request +per step; ``Notes`` records each revision. It is accepted when the last step +merges and the record describes what was built. + +Context +------- + +An adapter tells shared code about its service in three unconnected ways. + +**A name.** Six adapters pass a string such as ``adapter="waterdata"`` to +configuration, retry policy, pagination, and fan-out -- 18 call sites in 10 +files, each spelling the name again, plus the ``adapter`` attribute of each +``Configuration`` class. The same name is the key of the adapter's table in the +configuration file. + +**A URL.** Each adapter resolves its base URL from configuration in its own +way, and Water Data alone has four APIs under one root: OGC API Features, a +STAC catalog (ratings), Samples, and Statistics. Nothing in the code says these +are four endpoints of one service. + +**Hard-coded facts.** The shared OGC code lists Water Data's parameter names as +"the OGC date parameters", exempts ``last_modified`` by name from date-only +rendering, passes durations through for every service because one accepts +them, and assumes every service spells an open range start ``..``. ADR 0003 +keeps shared code from *importing* a service; it says nothing about *naming* +one, and a string literal is not an import. The services differ: + +.. list-table:: + :header-rows: 1 + + * - Behavior (observed 2026-10-11) + - Water Data OGC + - NGWMN + - Water Data STAC + * - Date parameters + - ``time``, ``begin``, ``end``, ``last_modified``, ``datetime`` + - ``datetime`` + - ``datetime`` + * - 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 + +Pull request #443 formatted interval strings to the one open-start spelling the +OGC code knew, which broke those requests on NGWMN and STAC. Its review missed +it because no rule said which facts belong to a service. + +Dates are also read and written in one function. Reading what a caller passed +(a string, a ``date``, a ``Timestamp``, ``None``, local-time rules) is package +policy; writing it as an ISO 8601 interval is protocol grammar. Every date +defect fixed in #440 through #443 was where those two meet. + +Decision +-------- + +Six abstractions, each answering one question: + +.. list-table:: + :header-rows: 1 + + * - Abstraction + - Answers + - Kind + * - **Service** + - Which external system: its name, and the URL its ``base_url`` setting + replaces. + - a value + * - **Endpoint** + - Which API of that service, at what URL, speaking which protocol with + which dialect. + - a value + * - **Protocol** + - How to talk to a standard API: request grammar, paging, filters, + chunking. + - functions + * - **Dialect** + - Where one endpoint departs from its protocol's standard. + - a value, one type per protocol + * - **Execution** + - How requests run: retry, fan-out, progress, quota (ADR 0006). + - functions + * - **Adapter** + - What the public sees: getters, parameter names, translation, docs. + - a module + +**An adapter declares one** ``Service`` **.** It holds the adapter's name and +its default URL, and it is the only place the adapter writes either. +Configuration and execution keep resolving settings by the service's name, +because that is the key a configuration file uses: they take ``service.name``, +read from the declaration, never a literal. Only resolving the URL takes the +value itself. + +**Each API protocol code reaches is an** ``Endpoint`` **of its service.** An +endpoint names its service, its URL, and the dialect of its protocol. The +adapter builds it when a call starts, from the configuration in effect, so +every chunk and a later resume use the same URL. Water Data is one service with +four endpoints. + +**A protocol is implemented as the standard is written.** Protocol code is +functions that receive an endpoint. Each dialect field records one way an +endpoint *extends* the standard (Water Data accepts a lone duration), +*restricts* it (NGWMN refuses ``../``), or *renames* it (Water Data filters on +``time``), and its default is the standard. A default dialect therefore +describes an endpoint that follows the standard. A field's docstring records the +observed server behavior that justifies it, and when it was observed. + +**Shared code names no service.** Nothing a ``Service``, ``Endpoint``, or +dialect declares -- a name, a parameter, a collection, a column -- appears as a +literal in protocol or execution code; that code reads it from the declaration. +A protocol's own standard names (``datetime``, ``bbox``, ``limit``) are not +deviations. A comment may cite a service as evidence for why the code tolerates +a variation; the behavior may not depend on which service it is. + +**Dialect fields are grouped by the code that reads them.** OGC's dialect has a +date group and a result-shaping group; request construction reads one field, +which stays on the dialect itself. A group is created when one part of the code +reads several fields together. + +**A date range is read once, at the public boundary, and written by each +protocol.** One package-wide value holds what the caller meant. The ISO 8601 +interval grammar shared by OGC and STAC is written by one module, adjusted by a +``DateDialect``; another protocol writes the value in its own format. + +**Values are data; protocols and execution are functions.** ``Service``, +``Endpoint``, and the dialect types are frozen dataclasses with no behavior. +They do not send requests or read configuration, and nothing subclasses them or +a protocol's code. A method that sends requests, or an override, could +change behavior for one service without declaring a field, which is the problem +this record exists to prevent. + +**Facts about one collection travel with the call.** Which column the wire +``id`` is renamed to, and whether a collection has geometry, are arguments of +each call, not fields of an endpoint: reference tables and ``get_cql`` reach +collections outside any adapter's map. + +**A protocol module exists only when two endpoints share the protocol.** OGC +API Features has two (Water Data, NGWMN). The interval grammar has three (those +two and STAC). WQP, NLDI, NWDC, and StreamStats keep their request code in their +adapters until a second endpoint shares it. + +Consequences +------------ + +- Adding a service means declaring values, not editing shared code, and a new + endpoint of an existing protocol starts from the standard's behavior. +- Everything shared code knows about a service is in one place, next to the + evidence for each deviation. +- Inside ``ogc``, one endpoint argument replaces the ``base_url``, ``dialect``, + and ``adapter`` arguments that are passed together today. +- A deviation the protocol code does not model needs a new field first. That + costs one field and its default, deliberately. +- Behavior change, when the date step lands: a lone duration sent to NGWMN + raises ``ValueError`` before sending instead of returning HTTP 400. + +Alternatives rejected: + +- *Per-service conditionals in shared code.* The current state. +- *A client class per service, or methods such as* ``WATERDATA.get_data()``. + A method that sends requests puts the protocol behind the value, which would + make the module that defines dialects import the OGC engine -- forbidden for + the modules that need the type -- and invites per-service overrides. +- *One OGC-specific service object holding name, URL, and dialect.* Name and + default URL are not OGC-specific: every adapter has them. One service can have + endpoints in several protocols. +- *A flat dialect.* The date fields are read together and are used without the + rest by STAC. +- *Probing a service at runtime for what it accepts.* That adds a request per + call to learn a fact that changes rarely, and some of these behaviors come + from a firewall the API does not describe. + +Compliance +---------- + +``tests/architecture_test.py`` fails if a service's name appears as a string +literal anywhere in the package except the configuration roster and that +service's declaration. Once endpoints and dialects land, it also fails if a +parameter, collection, or column a shipped dialect lists appears as a literal +in protocol code. Docstrings and comments are excluded. + +Each dialect field that records a server behavior has a ``@pytest.mark.live`` +test that sends the request the field prevents and the request it produces, so +the scheduled live workflow fails when a deployment changes. + +``CONTEXT.md`` defines *Endpoint*, *Protocol*, and *Dialect*, and says how a +*Service* is declared. + +Deviations still in the OGC protocol code, each removed when another endpoint +needs a different value or the code holding it next changes: + +- ``monitoring_location_id`` is validated by name against the ``AGENCY-ID`` form + both OGC services use. +- ``skipGeometry`` is pygeoapi's spelling; 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 review of #442 and #443. Supersedes the narrower proposal in +#444, which bundled name, URL, and dialect into one OGC-specific value. The +behaviors in the Context table were observed by sending each request to each +service on 2026-10-11. + +Implementation steps, each its own pull request: declare a ``Service`` per +adapter; endpoints for the OGC getters; endpoints for the remaining adapters; +the package-wide date range; the shared interval grammar and ``DateDialect``; +the OGC code naming nothing a dialect declares; acceptance. + +Revisions: + +- *Service declarations.* Each adapter declares its ``Service`` in the module + that owns its URL (Water Data's in ``waterdata.endpoints``, shared by its four + APIs). The type is a dependency-free leaf, ``dataretrieval._service``. + Configuration, execution, and the OGC code keep taking the service's *name* + rather than the value: settings are keyed by name, and a value would add + nothing they read. ``configuration.service_url`` resolves a service's URL. The + name check excludes ``__all__``, whose entries name modules that share a + service's spelling. +- *Endpoints for the OGC getters.* An endpoint holds a resolved URL rather than + a function that resolves one: the adapter builds it at the start of each + call, which gives the same once-per-call guarantee with no behavior in the + value. The OGC engine, request builder, and result finalizer take one + ``endpoint`` where they took ``base_url``, ``dialect``, and ``adapter``; + functions that need only the URL (schema and CQL requests) still take it + alone. ``DEFAULT_DIALECT`` is removed, since every call now supplies a + dialect through its endpoint. +- *Endpoints for the remaining adapters* -- not built. An ``Endpoint`` value + exists only where protocol code receives one, because its point is to carry a + dialect to code shared between endpoints. WQP, NLDI, NWDC, StreamStats, and + Water Data's Samples and Statistics APIs have no protocol module, so an + endpoint there would carry no dialect and be read only for its URL, which + ``configuration.service_url`` and ``waterdata.endpoints`` already resolve. They + are still endpoints in the glossary's sense. The STAC ratings search gets an + ``Endpoint`` with the interval grammar, where its ``DateDialect`` is read. +- *The package-wide date range.* ``dataretrieval._dates`` is a dependency-free + leaf holding :class:`DateRange` and ``read_date_range``: an instant, a range + whose ``None`` bound is open, or a duration, with a naive bound meaning local + time. ``prepare_request_args`` reads every date parameter of the + ``OgcDialect`` it is given into one when a getter is called, and drops a + range whose bounds are all open, as it drops ``None``; ``get_ratings`` reads + its ``time`` the same way. ``get_ogc_data`` reads any date parameter still + raw -- a verbatim ``query`` skips ``prepare_request_args`` -- with the + endpoint's dialect, and raises when a ``DateRange`` arrives under a key that + dialect does not list, so arguments read with one dialect are not sent with + another. The OGC request builder only writes the value. Error messages are + unchanged by this step; they are raised when the getter is called rather + than when the first request is built. +- *The shared interval grammar and* ``DateDialect``. ``dataretrieval._intervals`` + holds ``DateDialect`` and ``write_interval``, and sits below ``ogc`` because + STAC is not OGC; ``ogc.dates`` is removed. ``DateDialect`` holds the grammar + both protocols read, two fields: ``open_start``, and ``durations``, whose + default is ``False`` because neither standard defines a lone duration. The + fields only OGC reads -- ``date_params``, ``date_only_collections``, and + ``timed_params`` -- are fields of ``OgcDialect``, since only OGC has + collections and STAC names its one date parameter in ``get_ratings``. + ``OgcDialect.dates`` holds the grammar, and ``prepare_request_args`` takes + the ``OgcDialect``, so the reading side knows which parameters carry dates + and whether a duration is accepted: an endpoint that refuses one raises when + the getter is called. Given no dialect it reads no date, which is how the + Samples and Statistics getters call it, through their own argument helper. + ``write_interval`` never raises; the OGC request builder decides from + ``date_only_collections`` and ``timed_params`` whether it writes date-only + bounds. Two messages change: the duration refusal + is one shared message naming the caller's argument (``get_ratings`` had its + own), and the "too many values" message offers a duration only where one is + accepted. A field's observed server behavior is recorded where the field is + set, in each adapter's declaration, since the type is shared code and names + no service. ``get_ratings`` builds an ``Endpoint`` whose dialect is a + ``DateDialect`` alone; STAC has no other dialect fields. Live tests cover + ``open_start`` and ``durations`` for each of the three endpoints. + ``date_only_collections`` and ``timed_params`` have none: they record what a + ``daily`` value means rather than what a server refuses, since ``daily`` + accepts full datetimes too (observed 2026-10-11). Also observed: STAC reads + an empty start as open only before an RFC 3339 datetime + (``/2026-01-01T00:00:00Z``); ``/2026-01-01`` is HTTP 400. ``get_ratings`` + always writes datetimes. +- *The OGC code names nothing a dialect declares.* ``OgcDialect`` has the + date group, ``dates``, and the result-shaping group, ``shaping`` + (``ShapingDialect``: ``time_cols``, ``numerical_cols``, ``sort_cols``, and + ``extra_id_cols``, which moved there from an argument of ``get_ogc_data``). + Request construction reads ``lists_as_cql2`` (#446) and the three OGC date + fields, which sit on ``OgcDialect`` itself. The chunk planner lists no date + parameter: a date argument arrives as a ``DateRange``, which is never split. + ``tests/architecture_test.py`` fails if a string a shipped dialect lists + appears as a literal in ``ogc`` or ``_intervals``. Of the four + recorded deviations, only ``monitoring_location_id`` is such a literal, so + it is the one allowlisted entry. The switch is per API because whether the + Water Data API reads a comma-joined GET value as a list depends on the + parameter, not the collection, and it does so only for a few identifiers and + codes (probed 2026-10-11, v0 and v1); a live test checks that a list returns + the union of its values. 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..e02c6526 100644 --- a/docs/source/architecture/index.rst +++ b/docs/source/architecture/index.rst @@ -118,8 +118,9 @@ 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 + ``shaping``, ``filters``, and ``errors`` isolate their named protocol + concerns. Date parameters are written by ``dataretrieval._intervals``, the + interval grammar OGC shares with STAC. The full runtime OGC graph, including the facade, is acyclic -- enforced by the package-wide fitness function in ``tests/architecture_test.py``. diff --git a/tests/_dates_test.py b/tests/_dates_test.py new file mode 100644 index 00000000..ae75ba2f --- /dev/null +++ b/tests/_dates_test.py @@ -0,0 +1,78 @@ +"""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 + +NAIVE = datetime.datetime(2024, 1, 1) # noqa: DTZ001 -- a naive bound is the case +UTC = datetime.datetime(2024, 1, 1, tzinfo=datetime.timezone.utc) + + +@pytest.mark.parametrize( + "value, expected", + [ + ("2024-01-01", DateRange(start=NAIVE, instant=True)), + (datetime.date(2024, 1, 1), DateRange(start=NAIVE, instant=True)), + ("P7D", DateRange(duration="P7D")), + (["2024-01-01T00:00:00Z", None], DateRange(start=UTC)), + ("../2024-01-01", DateRange(end=NAIVE)), + ([pd.NaT, pd.Timestamp("2024-01-01")], DateRange(end=NAIVE)), + ], +) +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: a + one-element instant is not a range with an open end.""" + 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=NAIVE)} + 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, Statistics) gets its arguments as given.""" + assert prepare_request_args({"time": "2024-01-01"}) == {"time": "2024-01-01"} + + +def test_a_date_range_the_endpoint_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()) + + +def test_read_date_range_refuses_a_duration_where_none_is_accepted(): + """A duration the endpoint refuses raises when the getter is called, naming + the caller's argument; where durations are accepted it is kept as given.""" + with pytest.raises(ValueError, match=r"^time cannot be an ISO 8601 duration"): + read_date_range("P7D", name="time", durations=False) + assert read_date_range("P7D", name="time", durations=True) == DateRange( + duration="P7D" + ) diff --git a/tests/architecture_test.py b/tests/architecture_test.py index c098eb4d..5a1911c1 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 @@ -371,31 +372,15 @@ def test_credential_policy_has_one_definition() -> None: ``transport.http`` re-exports the predicate, it does not restate it. """ host = "api.waterdata.usgs.gov" - # Walked as AST string *values*, not as source text. A line-substring match - # produces both false negatives and false positives: it missed the - # ``"https://…"`` form three modules use to write the same host, and it - # flagged docstring prose that only names the service. Docstrings are - # excluded here (they are documentation, not a second definition) - # while every other literal -- bare host or full base URL -- counts. - offenders: list[str] = [] - for path in sorted(PACKAGE_ROOT.rglob("*.py")): - if path.name == "credentials.py": - continue - tree = ast.parse(path.read_text(encoding="utf-8")) - docstrings = { - text - for node in ast.walk(tree) - if isinstance( - node, (ast.Module, ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef) - ) - for text in [ast.get_docstring(node, clean=False)] - if text is not None - } - for node in ast.walk(tree): - if not isinstance(node, ast.Constant) or not isinstance(node.value, str): - continue - if host in node.value and node.value not in docstrings: - offenders.append(f"{_module_name(path)}:{node.lineno}") + # String *values*, not source text: a line match missed the ``"https://…"`` + # form and flagged docstring prose that only names the service. + offenders = [ + f"{_module_name(path)}:{line}" + for path in sorted(PACKAGE_ROOT.rglob("*.py")) + if path.name != "credentials.py" + for line, value in _code_string_literals(path) + if host in value + ] assert not offenders, ( "The API-key host must come from dataretrieval.credentials, " f"not be restated at: {offenders}" @@ -667,3 +652,129 @@ def test_adr_references_resolve_to_a_record() -> None: assert not dangling, "citations name no such decision record:\n " + "\n ".join( dangling ) + + +def _service_declarations() -> dict[str, list[Path]]: + """Each ``Service("name", ...)`` call in the package, by name.""" + found: dict[str, list[Path]] = {} + for path in sorted(PACKAGE_ROOT.rglob("*.py")): + for node in ast.walk(ast.parse(path.read_text(encoding="utf-8"))): + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id == "Service" + and node.args + and isinstance(node.args[0], ast.Constant) + ): + found.setdefault(node.args[0].value, []).append(path) + return found + + +def test_service_names_are_written_only_where_declared() -> None: + """ADR 0014: an adapter declares its service once, and everything else reads + the name from that declaration. A name written as a literal anywhere else is + a second spelling of the declaration, which is how shared code ends up + naming a service. The configuration roster is the one other place, because + a file table must be validated before its adapter is imported.""" + from dataretrieval._configuration_core import ADAPTERS + + declarations = _service_declarations() + assert sorted(declarations) == sorted(ADAPTERS), ( + "declare exactly one Service per configuration roster entry" + ) + duplicated = {name: paths for name, paths in declarations.items() if len(paths) > 1} + assert not duplicated, f"a service is declared once: {duplicated}" + + roster = PACKAGE_ROOT / "_configuration_core.py" + found = [ + f"{path.relative_to(PACKAGE_ROOT)}:{line} {value!r}" + for path in sorted(PACKAGE_ROOT.rglob("*.py")) + if path != roster + for line, value in _code_string_literals(path) + if value in declarations and path not in declarations[value] + ] + assert not found, f"read the name from the service declaration: {found}" + + +def _code_string_literals(path: Path) -> list[tuple[int, str]]: + """String constants in *path*, excluding docstrings and ``__all__``, whose + entries name modules and symbols rather than services.""" + 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 endpoint (ADR 0014). +_PROTOCOL_PATHS = ( + *sorted((PACKAGE_ROOT / "ogc").glob("*.py")), + PACKAGE_ROOT / "_intervals.py", +) + +# The deviations ADR 0014 records as still written in the OGC protocol code. +# Only this one is a literal a dialect also lists; the other three (the +# ``skipGeometry`` spelling, the 50000 ``limit`` cap, and the unquoted-number +# CQL check) name nothing a dialect declares. +_RECORDED_DEVIATIONS = frozenset({"monitoring_location_id"}) + + +def _declared_strings(value: object) -> set[str]: + """Every string a dialect value lists, through nested groups.""" + 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 + deviations.""" + 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_DEVIATIONS + ] + assert not found, f"declare it as a dialect field and read it there: {found}" diff --git a/tests/configuration_test.py b/tests/configuration_test.py index 01abc7f4..d92c2f51 100644 --- a/tests/configuration_test.py +++ b/tests/configuration_test.py @@ -1291,17 +1291,29 @@ def test_every_adapter_is_actually_wired_to_a_read_site(): package-wide without an error. """ import pathlib - - source = "\n".join( - p.read_text(encoding="utf-8") - for p in pathlib.Path(configuration.__file__).parent.rglob("*.py") - if p.name != "configuration.py" - ) - missing = [a for a in configuration.ADAPTERS if f'adapter="{a}"' not in source] + import re + + root = pathlib.Path(configuration.__file__).parent + # Each adapter declares its service once (ADR 0014) and reads the name from + # that declaration, so a read site is ``adapter=.name`` or an + # ``Endpoint(, ...)`` in the adapter's own module or package. + declared = re.compile(r'(_?SERVICE) = Service\("([a-z]+)"') + missing = list(configuration.ADAPTERS) + for path in root.rglob("*.py"): + for variable, name in declared.findall(path.read_text(encoding="utf-8")): + scope = path.parent.glob("*.py") if path.parent != root else [path] + source = "\n".join(p.read_text(encoding="utf-8") for p in scope) + reads = ( + f"adapter={variable}.name", + f"Endpoint(\n {variable},", + f"Endpoint({variable},", + ) + if any(r in source for r in reads) and name in missing: + missing.remove(name) assert not missing, ( f"adapters with a schema but no read site: {missing}. Either pass " - 'adapter="" where that adapter builds its policy or fan-out, or ' - "drop it from configuration.ADAPTERS." + "adapter=.name where that adapter builds its policy or " + "fan-out, or drop it from configuration.ADAPTERS." ) @@ -2228,3 +2240,17 @@ def test_show_configuration_reports_an_unresolvable_path_as_the_file_row( text = out.getvalue() assert "config file int: + headers = default_headers(url) + return httpx.get(url, params=params, headers=headers, timeout=60).status_code + + return status diff --git a/tests/contracts/README.md b/tests/contracts/README.md index c8d24e1d..0fe4c68c 100644 --- a/tests/contracts/README.md +++ b/tests/contracts/README.md @@ -10,7 +10,7 @@ The suite uses four dependency-oriented layers without moving established tests: response parsing, and documented protocol behavior. - **Component** (`transport_test.py`, `waterdata_chunking_test.py`, `waterdata_queryables_test.py`, `waterdata_endpoints_test.py`, - `waterdata_properties_test.py`, `rdb_test.py`, `_csv_test.py`): one internal + `waterdata_properties_test.py`, `rdb_test.py`, `_csv_test.py`, `_dates_test.py`): one internal responsibility in isolation. - **Cross-component** (`architecture_test.py`, `headers_host_scoping_test.py`, `waterdata_progress_test.py`): dependency fitness functions and behavior that diff --git a/tests/headers_host_scoping_test.py b/tests/headers_host_scoping_test.py index f5aeeb03..28e844c2 100644 --- a/tests/headers_host_scoping_test.py +++ b/tests/headers_host_scoping_test.py @@ -117,11 +117,13 @@ def handler(request: httpx.Request) -> httpx.Response: def test_generic_ogc_request_excludes_key_for_custom_host(self): """A caller-supplied OGC base URL never receives Water Data auth.""" + from dataretrieval._service import Endpoint, Service + from dataretrieval.ogc.policy import OgcDialect from dataretrieval.ogc.requests import _construct_api_requests - request = _construct_api_requests( - "things", base_url="https://features.example.org/ogcapi" - ) + url = "https://features.example.org/ogcapi" + endpoint = Endpoint(Service("ngwmn", url), url, OgcDialect()) + request = _construct_api_requests("things", endpoint=endpoint) assert "X-Api-Key" not in request.headers def test_rating_download_scopes_headers_to_asset_url(self): diff --git a/tests/ngwmn_test.py b/tests/ngwmn_test.py index d73ae0f2..bf5dc3df 100644 --- a/tests/ngwmn_test.py +++ b/tests/ngwmn_test.py @@ -11,6 +11,7 @@ cannot tell us, because the mock is what would need updating. """ +import datetime import re from urllib.parse import parse_qs, urlsplit @@ -460,6 +461,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) @@ -634,3 +649,37 @@ def queryables(collection): providers_q = queryables("providers") assert "state_name" in sites_q and "state" not in sites_q, sites_q assert "state" in providers_q and "state_name" not in providers_q, providers_q + + +def test_get_water_level_refuses_a_duration_before_sending(httpx_mock): + """NGWMN answers a lone duration with HTTP 400, so it raises locally.""" + with pytest.raises(ValueError, match=r"^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 on purpose: the 403 comes from a firewall the API does not + describe, so only the running service can show it has changed. + """ + 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: the standard's "../" is refused; the empty start is read 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 refused, so none is sent. + assert status("P7D") == 400 diff --git a/tests/waterdata_chunking_test.py b/tests/waterdata_chunking_test.py index b9689ee7..445c1b56 100644 --- a/tests/waterdata_chunking_test.py +++ b/tests/waterdata_chunking_test.py @@ -34,6 +34,8 @@ import dataretrieval from dataretrieval import configuration as _configuration +from dataretrieval._dates import read_date_range +from dataretrieval._service import Endpoint from dataretrieval.combining import ( _QUOTA_HEADER, _combine_chunk_frames, @@ -62,16 +64,15 @@ 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, _request_bytes, _safe_request_bytes, ) +from dataretrieval.ogc.policy import OgcDialect from dataretrieval.ogc.requests import ( _construct_api_requests as _construct_api_requests_explicit, ) @@ -84,12 +85,14 @@ retry_async as _retry, ) from dataretrieval.utils import HTTPX_DEFAULTS +from dataretrieval.waterdata.endpoints import SERVICE from dataretrieval.waterdata.utils import OGC_API_URL # The joint-planner stress test uses the real request builder; bind the # target API the way ``get_ogc_data`` does — explicitly, via ``partial``. _construct_api_requests = functools.partial( - _construct_api_requests_explicit, base_url=OGC_API_URL + _construct_api_requests_explicit, + endpoint=Endpoint(SERVICE, OGC_API_URL, OgcDialect()), ) @@ -148,19 +151,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_filters_test.py b/tests/waterdata_filters_test.py index 857b541d..5b830909 100644 --- a/tests/waterdata_filters_test.py +++ b/tests/waterdata_filters_test.py @@ -7,21 +7,25 @@ import pandas as pd import pytest +from dataretrieval._service import Endpoint from dataretrieval.ogc.filters import ( _check_numeric_filter_pitfall, _cql_text_literal, _split_top_level_or, ) +from dataretrieval.ogc.policy import OgcDialect from dataretrieval.ogc.requests import ( _construct_api_requests as _construct_api_requests_explicit, ) from dataretrieval.waterdata import get_continuous +from dataretrieval.waterdata.endpoints import SERVICE from dataretrieval.waterdata.utils import OGC_API_URL # Bind the target API the way ``get_ogc_data`` does — explicitly — so the # direct construction tests below exercise the real builder. _construct_api_requests = functools.partial( - _construct_api_requests_explicit, base_url=OGC_API_URL + _construct_api_requests_explicit, + endpoint=Endpoint(SERVICE, OGC_API_URL, OgcDialect()), ) diff --git a/tests/waterdata_ratings_test.py b/tests/waterdata_ratings_test.py index e85f9edc..a188386b 100644 --- a/tests/waterdata_ratings_test.py +++ b/tests/waterdata_ratings_test.py @@ -44,8 +44,9 @@ 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"): + """STAC answers a lone duration with HTTP 400, so it raises before sending, + naming the caller's argument rather than the wire's ``datetime``.""" + with pytest.raises(ValueError, match=r"^time cannot be an ISO 8601 duration"): get_ratings( monitoring_location_id="USGS-01104475", time="P7D", @@ -171,6 +172,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 @@ -192,6 +210,28 @@ def test_get_ratings_keeps_a_dotdot_open_bound_in_time(httpx_mock): assert params["datetime"] == ["2026-04-29T00:00:00Z/.."] +@pytest.mark.parametrize( + "time", + [ + pd.Timestamp("2026-04-29", tz="UTC"), + [pd.Timestamp("2026-04-29", tz="UTC"), None], + ], + 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.""" + 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() + datetime_param = parse_qs(urlsplit(str(request.url)).query)["datetime"][0] + assert datetime_param.startswith("2026-04-29T00:00:00Z") + + def test_get_ratings_rejects_an_unreadable_time_before_any_request(httpx_mock): """An unreadable bound used to drop the ``datetime`` filter, so the search silently returned every rating regardless of date.""" @@ -401,3 +441,28 @@ def test_stac_next_link_strips_embedded_credentials(httpx_mock): followed = httpx_mock.get_requests()[1] assert followed.url.userinfo == b"" assert followed.headers.get("Authorization") is None + + +@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. The 403 comes from + a firewall the API does not describe, so only the running service shows it.""" + 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: the standard's "../" is refused; the empty start is read 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 refused, so none is sent. + assert status("P7D") == 400 diff --git a/tests/waterdata_test.py b/tests/waterdata_test.py index 0400d9c3..6f3b4ddb 100644 --- a/tests/waterdata_test.py +++ b/tests/waterdata_test.py @@ -15,10 +15,12 @@ import dataretrieval from dataretrieval import configuration +from dataretrieval._service import Endpoint from dataretrieval.ogc.policy import OgcDialect from dataretrieval.ogc.requests import ( _check_monitoring_location_id, _normalize_str_iterable, + prepare_request_args, ) from dataretrieval.ogc.requests import ( _construct_api_requests as _construct_api_requests_explicit, @@ -46,7 +48,11 @@ get_stats_por, get_time_series_metadata, ) +from dataretrieval.waterdata.endpoints import SERVICE from dataretrieval.waterdata.types import _check_profiles +from dataretrieval.waterdata.utils import ( + _NO_NORMALIZE_PARAMS as _WATERDATA_NO_NORMALIZE, +) from dataretrieval.waterdata.utils import ( OGC_API_URL, WATERDATA_DIALECT, @@ -75,15 +81,23 @@ ) -# The direct request-construction unit tests below bypass ``get_ogc_data``, -# which is what normally binds the Water Data base URL and dialect (the OGC -# package names no collection of its own). Bind them here the same way the -# engine does — explicitly, via ``functools.partial`` — so the tests exercise -# the real Water Data behavior (monitoring-locations -> POST/CQL2; daily -> -# date-only time args). -_construct_api_requests = functools.partial( - _construct_api_requests_explicit, base_url=OGC_API_URL, dialect=WATERDATA_DIALECT -) +# The direct request-construction unit tests below bypass the getters and +# ``get_ogc_data``, which normally read the date arguments and bind the Water +# Data base URL and dialect (the OGC package names no collection of its own). +# Do both here so the tests exercise the real Water Data behavior +# (monitoring-locations -> POST/CQL2; daily -> date-only time args). +def _construct_api_requests(collection, **kwargs): + return _construct_api_requests_explicit( + collection, + endpoint=Endpoint(SERVICE, OGC_API_URL, WATERDATA_DIALECT), + **prepare_request_args( + kwargs, + dialect=WATERDATA_DIALECT, + extra_no_normalize=_WATERDATA_NO_NORMALIZE, + ), + ) + + _construct_cql_request = functools.partial( _construct_cql_request_explicit, base_url=OGC_API_URL ) @@ -631,8 +645,7 @@ def test_construct_api_requests_comma_joins_without_lists_as_cql2(): default) gets comma-joined GET values.""" req = _construct_api_requests_explicit( "things", - base_url=OGC_API_URL, - dialect=OgcDialect(), + endpoint=Endpoint(SERVICE, OGC_API_URL, OgcDialect()), monitoring_location_id=["USGS-A", "USGS-B"], filter="x = '1'", ) @@ -671,8 +684,7 @@ def test_construct_api_requests_keeps_comma_lists_beside_a_json_filter(): beside the JSON filter body.""" req = _construct_api_requests_explicit( "things", - base_url=OGC_API_URL, - dialect=OgcDialect(), + endpoint=Endpoint(SERVICE, OGC_API_URL, OgcDialect()), monitoring_location_id=["USGS-A", "USGS-B"], filter='{"op": "=", "args": [{"property": "x"}, "1"]}', filter_lang="cql-json", @@ -721,6 +733,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` @@ -732,10 +753,22 @@ def test_construct_api_requests_two_element_date_list_becomes_interval(): time=["2024-01-01", "2024-01-31"], ) assert req.method == "GET" - # `/` URL-encodes to %2F. Confirms _format_api_dates ran before the join. + # `/` URL-encodes to %2F. Confirms the date range was written before the join. assert "time=2024-01-01%2F2024-01-31" in str(req.url) +def test_construct_api_requests_accepts_timestamp_bounds(): + """A ``pandas.Timestamp`` bound, as in ``time=[df.index.min(), None]``, + builds the same request as its string spelling instead of raising + ``AttributeError``.""" + req = _construct_api_requests( + "daily", + monitoring_location_id="USGS-05427718", + time=[pd.Timestamp("2024-01-01", tz="UTC"), None], + ) + assert "time=2024-01-01%2F.." in str(req.url) + + # --- mocked getter smoke tests ------------------------------------------------ # These replace what used to be ~34 live calls to the Water Data API. Each one # serves a committed fixture (``tests/data/waterdata_ogc_fixtures.json``, two @@ -936,7 +969,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 @@ -1699,6 +1734,23 @@ 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; a raw string used to raise AttributeError.""" + _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_with_query(httpx_mock): """A ``query`` dict is merged into the request's query params.""" _mock_items(httpx_mock, "agency-codes") @@ -2021,6 +2073,33 @@ def test_get_reference_table_docstring_lists_every_collection(): assert not missing, f"collections served but undocumented: {missing}" +@pytest.mark.live +def test_ogc_date_dialect_matches_the_service(live_status): + """The ``WATERDATA_DIALECT.dates`` fields that record server behavior, + against the running API: it accepts the request each field produces and + refuses the one ``open_start`` prevents.""" + 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 refused. + 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 + ) + + @pytest.mark.live @pytest.mark.parametrize( "filters", diff --git a/tests/waterdata_utils_test.py b/tests/waterdata_utils_test.py index 281744c0..1b855c1f 100644 --- a/tests/waterdata_utils_test.py +++ b/tests/waterdata_utils_test.py @@ -3,6 +3,7 @@ import functools import json import logging +import re import warnings from unittest import mock @@ -13,6 +14,9 @@ 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._service import Endpoint, Service from dataretrieval.exceptions import ( DataRetrievalError, HTTPError, @@ -21,7 +25,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, @@ -31,7 +34,7 @@ _parse_retry_after, _raise_for_non_200, ) -from dataretrieval.ogc.policy import OgcDialect +from dataretrieval.ogc.policy import OgcDialect, ShapingDialect from dataretrieval.ogc.schema import _check_ogc_requests from dataretrieval.ogc.shaping import ( _arrange_cols, @@ -42,9 +45,9 @@ ) from dataretrieval.ogc.shaping import _finalize_ogc as _ogc_finalize from dataretrieval.waterdata import get_stats_date_range, get_stats_por +from dataretrieval.waterdata.endpoints import SERVICE from dataretrieval.waterdata.stats import _handle_nesting, get_data from dataretrieval.waterdata.utils import ( - _EXTRA_ID_COLS, OGC_API_URL, WATERDATA_DIALECT, _flatten_queryables, @@ -55,9 +58,7 @@ # exercise the same result shape the typed getters produce. _finalize_ogc = functools.partial( _ogc_finalize, - extra_id_cols=_EXTRA_ID_COLS, - dialect=WATERDATA_DIALECT, - base_url=OGC_API_URL, + endpoint=Endpoint(SERVICE, OGC_API_URL, WATERDATA_DIALECT), geopd=False, include_geometry=True, ) @@ -860,7 +861,7 @@ def test_arrange_cols_keeps_geometry_when_present(): # --- _type_cols -------------------------------------------------------------- -_TYPING_DIALECT = OgcDialect( +_TYPING_DIALECT = ShapingDialect( time_cols=frozenset({"time"}), numerical_cols=frozenset({"value"}) ) @@ -913,7 +914,17 @@ def test_type_cols_warning_is_singular_for_one_value(): _type_cols(df, _TYPING_DIALECT) -# --- _format_api_dates ------------------------------------------------------- +# --- read_date_range, then write_interval ----------------------------------- + + +def _wire_dates(value, date=False, *, name="date input", open_start=".."): + """Read a date argument and write it as an interval, as a getter and the + request builder do in turn; ``None`` when there is no filter. ``date`` + writes date-only bounds.""" + read = read_date_range(value, name=name, durations=True) + if read is None: + return None + return write_interval(read, DateDialect(open_start=open_start), date_only=date) @pytest.mark.parametrize( @@ -948,7 +959,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", @@ -961,25 +972,133 @@ def test_type_cols_warning_is_singular_for_one_value(): "open_ended_dotdot_datetime", ], ) -def test_format_api_dates(value, date, expected): - """``_format_api_dates`` normalizes ISO 8601 datetimes to UTC (dropping +def test_wire_dates(value, date, expected): + """``_wire_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 ``..``.""" - assert _format_api_dates(value, date=date) == expected + passes durations through unchanged, and renders a None endpoint as + ``..``.""" + assert _wire_dates(value, date=date) == expected -def test_format_api_dates_passes_none_through(): +def test_wire_dates_passes_none_through(): """``None`` means "no date filter", not an empty one.""" - assert _format_api_dates(None) is None + assert _wire_dates(None) is None -def test_format_api_dates_treats_an_all_blank_sequence_as_no_filter(): +def test_wire_dates_treats_an_all_blank_sequence_as_no_filter(): """``[None, None]`` is an interval with no endpoints, which is no filter at all -- sending ``../..`` would be a query the service has to reject.""" - assert _format_api_dates([None, None]) is None - assert _format_api_dates(["", ""]) is None - assert _format_api_dates(["..", ".."]) is None + assert _wire_dates([None, None]) is None + assert _wire_dates(["", ""]) is None + assert _wire_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_wire_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 _wire_dates(interval, date=date) == _wire_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_wire_dates_formats_each_side_of_an_interval_string(interval, date, expected): + """Each side is formatted on its own, as a list element is.""" + assert _wire_dates(interval, date=date) == expected + + +def test_wire_dates_treats_an_all_open_interval_string_as_no_filter(): + """``"../.."`` is the string spelling of ``[None, None]``.""" + assert _wire_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_wire_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 _wire_dates(value) == "../2024-01-01T00:00:00Z" + assert _wire_dates(value, open_start="") == "/2024-01-01T00:00:00Z" + + +def test_wire_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 = _wire_dates(["2024-01-01T00:00:00Z", None], open_start="") + assert open_end == "2024-01-01T00:00:00Z/.." + assert _wire_dates([None, None], open_start="") is None + assert _wire_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_read_date_range_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}"): + read_date_range(interval, name="time", durations=True) @pytest.mark.parametrize( @@ -994,59 +1113,131 @@ def test_format_api_dates_treats_an_all_blank_sequence_as_no_filter(): "range_start", ], ) -def test_format_api_dates_rejects_an_unreadable_bound(value): +def test_read_date_range_rejects_an_unreadable_bound(value): """An element that is not blank and matches no format used to return None, which the callers send as no filter at all. The message names the caller's argument and the bad value, and shows the forms that are accepted.""" with pytest.raises(ValueError) as excinfo: - _format_api_dates(value, name="last_modified") + read_date_range(value, name="last_modified", durations=True) message = str(excinfo.value) assert message.startswith("last_modified could not be read as a date") assert "'2024-01-01'" in message assert "None for an open end" in message -def test_format_api_dates_rejects_more_than_two_values(): +def test_read_date_range_rejects_more_than_two_values(): """A date filter is an instant, a duration, or a closed interval. Three values is a caller who meant something else, and the message says which shapes exist rather than truncating without an error.""" with pytest.raises(ValueError) as excinfo: - _format_api_dates(["2024-01-01", "2024-06-01", "2024-12-31"], name="time") + read_date_range( + ["2024-01-01", "2024-06-01", "2024-12-31"], name="time", durations=True + ) message = str(excinfo.value) assert "time takes at most 2 values, got 3" in message assert "closed interval" in message def test_the_duration_example_is_withheld_where_durations_are_rejected(): - """``get_ratings`` refuses ISO 8601 durations, so the shared message must not offer + """Where durations are refused (STAC, NGWMN), the message must not offer 'P7D' on that path -- 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) -def test_format_api_dates_names_the_callers_parameter(): +def test_read_date_range_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 accepts, so correcting the argument the message named sent an unrecognized parameter.""" - with pytest.raises(TypeError, match="^time must be a string"): - _format_api_dates({"2024-01-01": "ignored"}, name="time") + with pytest.raises(TypeError, match="^time must be a date, a string"): + read_date_range({"2024-01-01": "ignored"}, name="time", durations=True) -def test_format_api_dates_rejects_mapping(): - """`time={"2024-01-01": "x"}` would materialize as the keys list, - accepting input the user clearly didn't intend. - """ - with pytest.raises(TypeError, match="date input must be a string or sequence"): - _format_api_dates({"2024-01-01": "ignored"}) +@pytest.mark.parametrize( + "value, date, expected", + [ + (datetime.date(2024, 1, 1), True, "2024-01-01"), + ([datetime.date(2024, 1, 1), None], True, "2024-01-01/.."), + ( + [datetime.date(2024, 1, 1), datetime.date(2024, 2, 1)], + True, + "2024-01-01/2024-02-01", + ), + (pd.Timestamp("2024-01-01T10:30:00", tz="UTC"), True, "2024-01-01"), + ( + pd.Timestamp("2024-01-01T10:30:00", tz="UTC"), + False, + "2024-01-01T10:30:00Z", + ), + ( + datetime.datetime( + 2024, 1, 1, 6, 0, tzinfo=datetime.timezone(datetime.timedelta(hours=-4)) + ), + False, + "2024-01-01T10:00:00Z", + ), + ( + [pd.Timestamp("2024-01-01", tz="UTC"), pd.NaT], + False, + "2024-01-01T00:00:00Z/..", + ), + ( + ["2024-01-01T00:00:00Z", pd.Timestamp("2024-02-01", tz="UTC")], + False, + "2024-01-01T00:00:00Z/2024-02-01T00:00:00Z", + ), + ], + ids=[ + "date", + "date_open_end", + "date_pair", + "aware_timestamp_date_only", + "aware_timestamp_to_utc", + "aware_datetime_offset_to_utc", + "nat_is_an_open_bound", + "mixed_string_and_timestamp", + ], +) +def test_wire_dates_accepts_date_and_datetime_objects(value, date, expected): + """``datetime.date``, ``datetime.datetime`` and ``pandas.Timestamp`` used to + raise ``AttributeError`` (no ``endswith``) or ``TypeError`` (a lone datetime + is not iterable) from inside the formatter. They are read like the + equivalent string, and ``NaT`` is an open bound like ``None``.""" + assert _wire_dates(value, date=date) == expected + + +@pytest.mark.parametrize("date", [True, False], ids=["date_only", "datetime"]) +def test_wire_dates_reads_naive_objects_like_naive_strings(date): + """A naive ``datetime``/``Timestamp`` is local time, the same rule a naive + string follows, so both spellings of one instant build the same filter in + any time zone. A ``date`` is midnight of that day, like ``"2024-01-01"``.""" + as_string = _wire_dates(["2024-01-01T10:00:00", None], date=date) + naive_datetime = [datetime.datetime(2024, 1, 1, 10), None] # noqa: DTZ001 + assert _wire_dates(naive_datetime, date=date) == as_string + naive_timestamp = [pd.Timestamp("2024-01-01T10:00:00"), None] + assert _wire_dates(naive_timestamp, date=date) == as_string + assert _wire_dates(datetime.date(2024, 1, 1), date=date) == ( + _wire_dates("2024-01-01", date=date) + ) + + +@pytest.mark.parametrize("value", [20240101, [20240101, None], 1.5]) +def test_read_date_range_rejects_other_types_naming_the_argument(value): + """A number is not a date. It used to fail with an ``AttributeError`` from + inside the formatter; the message now names the caller's argument and the + types it accepts.""" + with pytest.raises(ValueError) as excinfo: + read_date_range(value, name="time", durations=True) + message = str(excinfo.value) + assert message.startswith("time could not be read as a date") + assert "pandas.Timestamp" in message def _make_response(status, body, reason=None, content_type="text/html"): @@ -1581,3 +1772,63 @@ def test_a_json_scalar_body_yields_no_detail(self): assert _json_error_detail(httpx.Response(400, json="just a string")) is None assert _json_error_detail(httpx.Response(400, json=[1, 2, 3])) is None + + +def test_get_ogc_data_reads_everything_about_the_service_from_its_endpoint( + httpx_mock, +): + """ADR 0014: an endpoint is all the OGC engine is told about a service. The + request goes to its URL, its dialect decides the request shape (a + list is POSTed under ``lists_as_cql2``), and settings resolve by its + service's name.""" + import dataretrieval + from dataretrieval.ngwmn import NgwmnConfiguration + from dataretrieval.ogc.engine import get_ogc_data + + endpoint = Endpoint( + Service("ngwmn", "https://unused.example"), + "https://mirror.example/ogc", + OgcDialect(lists_as_cql2=True), + ) + httpx_mock.add_response( + method="POST", + url=re.compile(r"^https://mirror\.example/ogc/collections/things/items"), + json={"type": "FeatureCollection", "features": [], "numberReturned": 0}, + ) + with dataretrieval.configure(NgwmnConfiguration(retries=0)): + df, _ = get_ogc_data( + {"properties": ["a"], "site": ["A", "B"]}, + "things", + "thing_id", + endpoint=endpoint, + spatial=False, + ) + assert df.empty + (request,) = httpx_mock.get_requests() + assert request.method == "POST" + + +def test_a_resumed_call_keeps_the_url_its_endpoint_was_built_with(httpx_mock): + """The endpoint is built when the call starts (ADR 0014), so a call + interrupted inside a ``configure(base_url=...)`` block and resumed after the + block has exited still reaches the mirror it started on.""" + import dataretrieval + from dataretrieval.interruptions import FanOutInterrupted + from dataretrieval.waterdata import WaterdataConfiguration, get_daily + + mirror = re.compile(r"^https://mirror\.example/ogcapi/v1/collections/daily/items") + empty = {"type": "FeatureCollection", "features": [], "numberReturned": 0} + httpx_mock.add_response(url=mirror, status_code=429) + httpx_mock.add_response(url=mirror, json=empty) + + with dataretrieval.configure( + WaterdataConfiguration(base_url="https://mirror.example") + ): + with pytest.raises(FanOutInterrupted) as excinfo: + get_daily(monitoring_location_id="USGS-01646500", properties=["value"]) + excinfo.value.call.resume() + + assert [r.url.host for r in httpx_mock.get_requests()] == [ + "mirror.example", + "mirror.example", + ]