diff --git a/.importlinter b/.importlinter index 8379e909..3330effc 100644 --- a/.importlinter +++ b/.importlinter @@ -33,6 +33,9 @@ layers = _ambient | _response_metadata | codes | combining | interruptions | rdb credentials exceptions +; The advisory mechanism sits below the taxonomy it reads no names from: it +; imports nothing first-party, so any layer can warn without reaching sideways. + _deprecation ; 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/NEWS.md b/NEWS.md index 2f1b52d2..b4abd5ec 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,5 @@ +**08/13/2026:** Warning categories now say what they mean. Two advisories about *upstream data* were emitted as `DeprecationWarning` and as an uncategorized `warnings.warn` respectively; both are now `DataCurrencyWarning` (a `UserWarning` subclass, exported as `dataretrieval.DataCurrencyWarning`). **Behavior change:** WQP's legacy-WQX notice moves from `DeprecationWarning` to `DataCurrencyWarning`. Because `legacy=True` is the default on every WQP getter and the notice is unconditional, a downstream project running `-W error::DeprecationWarning` previously could not call any WQP getter with default arguments; it now can. The flip side is visibility — `DeprecationWarning` is silent by default outside `__main__`, so this notice will now print to stderr for library and notebook callers who never saw it. Silence it with `warnings.filterwarnings("ignore", category=dataretrieval.DataCurrencyWarning)`, or set `legacy=False` where a WQX3.0 profile exists. The NWIS qw-endpoint retirement notice gains the same category (it previously had none, arriving as a bare `UserWarning` that deprecation filters ignored). `DeprecationWarning` now means only that a name in this package is going away, always with a replacement and, where published, a removal date; those horizons are declared once in `dataretrieval._deprecation.REMOVALS` rather than spelled at each call site. + **08/09/2026:** `waterdata.get_cql` takes `collection` rather than `service`. OGC API - Features (17-069r4) normatively names this value the `collectionId`: Requirement 20 fixes the path template `/collections/{collectionId}/items`, and Requirement 18 defines `collectionId` as each `id` in the collections response -- which is literally how the package builds the URL, and what the live API returns. *Service* names the API itself (Water Data, NGWMN). **Deprecation:** `service=` still works and resolves to `collection`, with a `DeprecationWarning`; it will be removed on or after 2027-08-09. Positional callers (`get_cql("daily", cql)`) are unaffected. The `WATERDATA_SERVICES` type alias is now `WATERDATA_COLLECTIONS`, with `WATERDATA_SERVICES` retained as a permanent alias for the same object. Terms are defined in `CONTEXT.md`. **08/09/2026:** Every retrieval path now runs through one executor. `waterdata.get_cql` (via the OGC `fetch_ogc_request`) and `waterdata.get_stats_por` / `get_stats_date_range` (via the Statistics page walk) previously bypassed `dataretrieval.transport.fanout.FanOut` through a private sync bridge, which meant they were the only getters in the package with **no retry**: a mid-page-walk 429 or 503 failed the whole call while every typed getter and Water Use rode it out. Both now run as a one-item fan-out and the 25-line `transport/sync.py` is gone. **Behavior change:** those three getters now retry transient failures (`API_USGS_RETRIES`, default 4) and, when the retries are exhausted, raise the resumable `ServiceInterrupted` / `QuotaExhausted` rather than `ServiceUnavailable` / `RateLimited` / `NetworkError` — all remain `DataRetrievalError`, so broad handlers are unaffected, but narrow handlers around those calls must widen, and `.call.resume()` is now available on the interruption. A failure that retrying cannot fix (bad scheme, a hostname that does not resolve) still surfaces as `NetworkError` immediately. The progress line moved with it: `FanOut.resume()` opens the reporter it ticks into, so a driver can no longer run the shared executor and silently print nothing, and a `.call.resume()` fired long after the interruption now reports progress instead of running mute. Internal tidying with no public effect: the WQX3 / legacy-WQP CSV datetime shaping moved out of `dataretrieval.utils` (whose docstring reserves it for non-service-specific shaping) into the `dataretrieval._wqx` leaf; the five Water Data endpoint URLs are declared once in `dataretrieval.waterdata.endpoints` instead of being derived in three modules; the OGC queryables document is parsed by `dataretrieval.ogc.schema` so every OGC adapter can offer the table, with `waterdata.get_queryables` unchanged as its documented wrapper; and `ogc/engine.py` imports each symbol from the module that defines it. diff --git a/dataretrieval/__init__.py b/dataretrieval/__init__.py index 8ecfb7bf..2ed3d415 100644 --- a/dataretrieval/__init__.py +++ b/dataretrieval/__init__.py @@ -34,6 +34,7 @@ from dataretrieval.exceptions import ( ConfigurationError, + DataCurrencyWarning, DataRetrievalError, HTTPError, NetworkError, @@ -90,6 +91,7 @@ # so callers can ``except dataretrieval.DataRetrievalError`` "exceptions", "ConfigurationError", + "DataCurrencyWarning", "DataRetrievalError", "HTTPError", "NetworkError", diff --git a/dataretrieval/_deprecation.py b/dataretrieval/_deprecation.py new file mode 100644 index 00000000..fb29c39c --- /dev/null +++ b/dataretrieval/_deprecation.py @@ -0,0 +1,71 @@ +"""One advisory mechanism, and one place to read the removal horizons. + +Four spellings of "tell the caller something is going away" had grown up +independently -- a dated decorator in :mod:`~dataretrieval.nwis`, an undated +kwarg shim in :mod:`~dataretrieval.waterdata.utils`, an undated module-level +notice in :mod:`~dataretrieval.wqp`, and one bare :func:`warnings.warn` with +no category at all. Only one carried a date, so the horizons could not be +audited or bumped in one place, and the category was a per-author choice. + +The category matters more than the wording. A ``DeprecationWarning`` is a +promise that a *name in this package* is going away, so a downstream project +running ``-W error::DeprecationWarning`` is right to fail on it. An advisory +that an upstream *dataset* has stopped being updated is not that -- the API is +fine and the caller has nothing to migrate to -- and it belongs under +:class:`~dataretrieval.exceptions.DataCurrencyWarning` instead. See +:data:`REMOVALS` for the horizons this package has published. +""" + +from __future__ import annotations + +import warnings + +#: Published removal horizons, by the surface each covers. A date here is a +#: commitment already made in a released warning message; read it rather than +#: spelling a date at the call site, so bumping one is a single edit. +REMOVALS: dict[str, str] = { + "nwis": "2027-05-06", + "waterdata.get_cql(service=)": "2027-08-09", +} + + +def warn_deprecated( + subject: str, + *, + replacement: str, + removal: str | None = None, + detail: str = "", + stacklevel: int = 2, +) -> None: + """Emit this package's one deprecation advisory. + + Parameters + ---------- + subject + What is going away, as the caller spells it (``"nwis.get_dv"``, the + keyword ``"stateFips"``). + replacement + What to use instead. Named in every message because a deprecation + without a migration path is only an inconvenience. + removal + Date from :data:`REMOVALS`, or ``None`` when no horizon has been + published -- which reads as "a future release" rather than inventing + a commitment. + detail + Optional sentence appended after the advisory, for a rename whose + reason is worth giving. Appended, never interpolated into the + message, so a multi-sentence detail cannot corrupt the wording. + stacklevel + Frames to skip so the warning is attributed to the caller's own line, + not to this function. + """ + horizon = f"on or after {removal}" if removal else "in a future release" + message = ( + f"{subject} is deprecated and will be removed from `dataretrieval` " + f"{horizon}; use {replacement} instead." + ) + warnings.warn( + f"{message} {detail}" if detail else message, + DeprecationWarning, + stacklevel=stacklevel + 1, + ) diff --git a/dataretrieval/exceptions.py b/dataretrieval/exceptions.py index a6033893..f71be7df 100644 --- a/dataretrieval/exceptions.py +++ b/dataretrieval/exceptions.py @@ -15,7 +15,9 @@ :class:`NoSitesError`, and :class:`ConfigurationError` for an unusable setting. :func:`error_for_status` maps a status to its type. The *warning* side of the taxonomy lives here too: :class:`SkippedItemWarning` (specialized by -:class:`SkippedRatingWarning`), a per-item skip inside a batched retrieval. +:class:`SkippedRatingWarning`) for a per-item skip inside a batched +retrieval, and :class:`DataCurrencyWarning` for an upstream dataset that has +stopped being updated. This module has no third-party runtime dependencies -- ``httpx`` is imported only for type checking. Any module can therefore import it without pulling in pandas @@ -44,6 +46,7 @@ "NetworkError", "NoSitesError", "ConfigurationError", + "DataCurrencyWarning", "SkippedItemWarning", "SkippedRatingWarning", "error_for_status", @@ -293,6 +296,24 @@ def __str__(self) -> str: ) +# --- Upstream data currency ----------------------------------------------- + + +class DataCurrencyWarning(UserWarning): + """An upstream dataset is frozen, retired, or no longer updated. + + Distinct from ``DeprecationWarning``, which promises that a *name in this + package* is going away and gives the caller something to migrate to. Here + the API is fine and there is nothing to migrate: the service's own data + has stopped moving, and only the caller can judge whether that matters. + + It is a ``UserWarning`` for that reason. Emitting it as a + ``DeprecationWarning`` meant a downstream project running + ``-W error::DeprecationWarning`` -- ordinary CI hygiene -- could not call + the affected getters with their default arguments at all. + """ + + # --- Skipped work --------------------------------------------------------- diff --git a/dataretrieval/nwis.py b/dataretrieval/nwis.py index e8354d71..2e1f4b13 100644 --- a/dataretrieval/nwis.py +++ b/dataretrieval/nwis.py @@ -16,7 +16,9 @@ import httpx import pandas as pd +from dataretrieval._deprecation import REMOVALS, warn_deprecated from dataretrieval._response_metadata import BaseMetadata +from dataretrieval.exceptions import DataCurrencyWarning from dataretrieval.rdb import read_rdb from ._querying import query @@ -51,7 +53,7 @@ } -_NWIS_REMOVAL_DATE = "2027-05-06" +_NWIS_REMOVAL_DATE = REMOVALS["nwis"] _REPLACEMENTS = { "get_dv": "`waterdata.get_daily()`", "get_iv": "`waterdata.get_continuous()`", @@ -70,11 +72,10 @@ def _warn_deprecated(func_name: str) -> None: """Emit a per-function DeprecationWarning pointing at the waterdata replacement.""" - warnings.warn( - f"`nwis.{func_name}` is deprecated and will be removed from " - f"`dataretrieval` on or after {_NWIS_REMOVAL_DATE}; " - f"use {_REPLACEMENTS[func_name]} instead.", - DeprecationWarning, + warn_deprecated( + f"`nwis.{func_name}`", + replacement=_REPLACEMENTS[func_name], + removal=_NWIS_REMOVAL_DATE, stacklevel=3, ) @@ -648,12 +649,13 @@ def get_info( if seriesCatalogOutput in ["True", "TRUE", "true", True]: warnings.warn( ( - "WARNING: Starting in March 2024, the NWIS qw data endpoint is " + "Starting in March 2024, the NWIS qw data endpoint is " "retiring and no longer receives updates. For more information, " "refer to https://waterdata.usgs.gov/nwis/qwdata and " "https://doi-usgs.github.io/dataRetrieval/articles/Status.html " "or email CompTools@usgs.gov." ), + DataCurrencyWarning, stacklevel=2, ) # convert bool to string if necessary diff --git a/dataretrieval/waterdata/cql.py b/dataretrieval/waterdata/cql.py index 1da51d75..2526dcf3 100644 --- a/dataretrieval/waterdata/cql.py +++ b/dataretrieval/waterdata/cql.py @@ -14,6 +14,7 @@ import pandas as pd +from dataretrieval._deprecation import REMOVALS from dataretrieval.waterdata.utils import ( _OUTPUT_ID_BY_COLLECTION, _accept_legacy_kwargs, @@ -30,11 +31,11 @@ @_accept_legacy_kwargs( {"service": "collection"}, + removal=REMOVALS["waterdata.get_cql(service=)"], detail=( "OGC API - Features names this value the collectionId (17-069r4 " "Requirements 18 and 20, /collections/{id}/items), while `service` " - "names the API itself (Water Data, NGWMN). `service` will be removed " - "on or after 2027-08-09." + "names the API itself (Water Data, NGWMN)." ), ) def get_cql( diff --git a/dataretrieval/waterdata/utils.py b/dataretrieval/waterdata/utils.py index d2cc9182..e001b877 100644 --- a/dataretrieval/waterdata/utils.py +++ b/dataretrieval/waterdata/utils.py @@ -17,12 +17,12 @@ from __future__ import annotations import functools -import warnings from collections.abc import Callable, Mapping from typing import TYPE_CHECKING, Any, TypeVar import pandas as pd +from dataretrieval._deprecation import warn_deprecated from dataretrieval.codes.states import apply_state from dataretrieval.ogc import OgcDialect, prepare_request_args from dataretrieval.ogc import get_ogc_data as _facade_get_ogc_data @@ -234,6 +234,7 @@ def _accept_legacy_kwargs( mapping: Mapping[str, str], *, detail: str = "", + removal: str | None = None, ) -> Callable[[Callable[..., _R]], Callable[..., _R]]: """Accept deprecated keyword-argument names on the decorated function. @@ -251,7 +252,10 @@ def _accept_legacy_kwargs( intentionally relaxed (the wrapper accepts the extra deprecated names), so static checkers won't flag legacy call sites. - ``detail`` appends a sentence to the warning. The default message says only + ``removal`` is the published horizon (from + :data:`~dataretrieval._deprecation.REMOVALS`); ``None`` reads as "a future + release". ``detail`` appends a sentence to the warning. The default + message says only that the name changed; a rename with a reason worth giving -- a spec that names the value differently, a removal date -- passes it here rather than hand-rolling the whole shim to carry one sentence. @@ -275,13 +279,11 @@ def wrapper(*args: Any, **kwargs: Any) -> _R: f"{func.__name__}() received both {old_name!r} " f"(deprecated) and {new_name!r}; pass only {new_name!r}." ) - message = ( - f"The {old_name!r} argument is deprecated and will be " - f"removed in a future release; use {new_name!r} instead." - ) - warnings.warn( - f"{message} {detail}" if detail else message, - DeprecationWarning, + warn_deprecated( + f"The {old_name!r} argument", + replacement=repr(new_name), + removal=removal, + detail=detail, stacklevel=2, ) kwargs[new_name] = kwargs.pop(old_name) diff --git a/dataretrieval/wqp.py b/dataretrieval/wqp.py index 2a41fd96..2d21acd9 100644 --- a/dataretrieval/wqp.py +++ b/dataretrieval/wqp.py @@ -17,6 +17,7 @@ import pandas as pd from dataretrieval._response_metadata import BaseMetadata +from dataretrieval.exceptions import DataCurrencyWarning from ._querying import _query_with_retry from ._wqx import _attach_datetime_columns @@ -732,7 +733,7 @@ def _warn_legacy_use() -> None: "information on updated WQX3.0 profiles. Setting `legacy=False` " "will remove this warning." ) - warnings.warn(message, DeprecationWarning, stacklevel=2) + warnings.warn(message, DataCurrencyWarning, stacklevel=2) def _warn_wqx3_unavailable() -> None: @@ -750,12 +751,13 @@ def _legacy_only_url(service: str, legacy: bool) -> str: Passing ``legacy=False`` to one of these helpers emits a ``UserWarning`` explaining the fallback and *also* suppresses the legacy - ``DeprecationWarning`` that ``wqp_url`` would otherwise raise. That - warning's message claims setting ``legacy=False`` removes it, which is - a lie for endpoints that have no WQX3.0 alternative. + :class:`~dataretrieval.exceptions.DataCurrencyWarning` that ``wqp_url`` + would otherwise raise. That warning's message claims setting + ``legacy=False`` removes it, which is a lie for endpoints that have no + WQX3.0 alternative. """ with warnings.catch_warnings(): if not legacy: _warn_wqx3_unavailable() - warnings.simplefilter("ignore", DeprecationWarning) + warnings.simplefilter("ignore", DataCurrencyWarning) return wqp_url(service) diff --git a/tests/deprecation_test.py b/tests/deprecation_test.py new file mode 100644 index 00000000..94a0b2b5 --- /dev/null +++ b/tests/deprecation_test.py @@ -0,0 +1,72 @@ +"""The behavioural claim: downstream CI hygiene must not break the library.""" + +import warnings + +import pytest + +import dataretrieval.wqp as wqp +from dataretrieval._deprecation import REMOVALS, warn_deprecated +from dataretrieval.exceptions import DataCurrencyWarning + + +def test_default_wqp_calls_survive_error_on_deprecationwarning(): + """A downstream project running ``-W error::DeprecationWarning`` -- ordinary + CI hygiene -- must still be able to call wqp with default arguments. + + ``legacy=True`` is the default on every wqp getter and ``wqp_url`` warns + unconditionally, so emitting that advisory as a ``DeprecationWarning`` + made the whole adapter uncallable under that filter. + """ + with warnings.catch_warnings(): + warnings.filterwarnings("error", category=DeprecationWarning) + with pytest.warns(DataCurrencyWarning): + wqp.wqp_url("Result") + + +def test_data_currency_is_not_a_deprecation(): + """The two categories must stay independently filterable: silencing stale + data must not silence a real removal notice, or vice versa.""" + assert not issubclass(DataCurrencyWarning, DeprecationWarning) + assert issubclass(DataCurrencyWarning, UserWarning) + + +def test_warn_deprecated_names_replacement_and_horizon(): + with pytest.warns( + DeprecationWarning, match=r"on or after 2027-05-06.*use `x` instead" + ): + warn_deprecated("`nwis.get_dv`", replacement="`x`", removal=REMOVALS["nwis"]) + + +def test_warn_deprecated_without_a_date_promises_nothing_specific(): + with pytest.warns(DeprecationWarning, match="in a future release"): + warn_deprecated("The 'a' argument", replacement="'b'") + + +def test_legacy_only_url_does_not_advise_setting_a_flag_already_set(): + """``what_*(legacy=False)`` must not be told to set ``legacy=False``. + + ``_legacy_only_url`` suppresses the legacy advisory for endpoints with no + WQX3.0 equivalent. The suppression names a category, so it has to follow + the advisory when the advisory's category changes. + """ + with warnings.catch_warnings(record=True) as rec: + warnings.simplefilter("always") + wqp._legacy_only_url("Station", False) + assert not [w for w in rec if issubclass(w.category, DataCurrencyWarning)] + assert [w for w in rec if "WQX3.0 profile not available" in str(w.message)] + + +def test_detail_is_appended_not_interpolated(): + """A multi-sentence ``detail`` must not be spliced inside the advisory.""" + with warnings.catch_warnings(record=True) as rec: + warnings.simplefilter("always") + warn_deprecated( + "The 'a' argument", + replacement="'b'", + removal="2027-01-01", + detail="Because reasons. And more.", + ) + message = str(rec[0].message) + assert message.endswith("Because reasons. And more.") + assert "use 'b' instead." in message + assert "in a future release" not in message diff --git a/tests/wqp_test.py b/tests/wqp_test.py index cde3c84e..8d289706 100644 --- a/tests/wqp_test.py +++ b/tests/wqp_test.py @@ -5,6 +5,7 @@ from pandas import DataFrame import dataretrieval.wqp as wqp +from dataretrieval.exceptions import DataCurrencyWarning from dataretrieval.wqp import ( WQP_Metadata, _check_kwargs, @@ -126,7 +127,7 @@ def test_get_results_WQX3(httpx_mock): wqp.wqp_url, "Result", "https://www.waterqualitydata.us/data/Result/Search?", - DeprecationWarning, + DataCurrencyWarning, ), ( wqp.wqx3_url, @@ -145,7 +146,7 @@ def test_wqp_url_profiles(builder, service, expected, warning): @pytest.mark.parametrize( ("builder", "profile", "valid_services", "warning"), [ - (wqp.wqp_url, "Legacy", wqp.services_legacy, DeprecationWarning), + (wqp.wqp_url, "Legacy", wqp.services_legacy, DataCurrencyWarning), (wqp.wqx3_url, "WQX3.0", wqp.services_wqx3, UserWarning), ], )