Skip to content

Deprecate remaining active nwis functions ahead of 2027-05-06 removal#271

Merged
thodson-usgs merged 4 commits intoDOI-USGS:mainfrom
thodson-usgs:deprecate-nwis
May 7, 2026
Merged

Deprecate remaining active nwis functions ahead of 2027-05-06 removal#271
thodson-usgs merged 4 commits intoDOI-USGS:mainfrom
thodson-usgs:deprecate-nwis

Conversation

@thodson-usgs
Copy link
Copy Markdown
Collaborator

@thodson-usgs thodson-usgs commented May 6, 2026 •
edited
Loading

Per the discussion on #267, every active nwis.* function now has a waterdata replacement, so each one should emit a DeprecationWarning with actionable migration guidance. The module-level "use waterdata instead" warning has been firing on import for a while; this PR makes the per-function guidance specific.

What changes

Each of the ten active nwis.* functions emits a DeprecationWarning on call, naming the specific waterdata replacement:

nwis function replacement
get_dv waterdata.get_daily()
get_iv waterdata.get_continuous()
get_info waterdata.get_monitoring_locations()
what_sites waterdata.get_monitoring_locations()
get_stats waterdata.get_stats_por() / waterdata.get_stats_date_range()
get_discharge_peaks waterdata.get_peaks()
get_ratings waterdata.get_ratings()
get_record the appropriate waterdata.get_*() for the service you need
query_waterdata a high-level waterdata.get_*() helper
query_waterservices a high-level waterdata.get_*() helper

(The five already-defunct functions — get_qwdata, get_discharge_measurements, get_gwlevels, get_pmcodes, get_water_use — already raise NameError and are unaffected.)

Implementation

Each of the ten getters is wrapped by an @_deprecated decorator that:

  1. Validates func.__name__ in _REPLACEMENTS at import time (typo → RuntimeError on import, not KeyError on first user call).
  2. Emits the DeprecationWarning with stacklevel pointing at the user's call site, not the wrapper.
  3. Suppresses re-entrant warnings via a threading.local() flag, so wrappers like get_record → get_iv → query_waterservices surface only the outermost call's warning instead of three near-identical messages.
_NWIS_REMOVAL_DATE = "2027-05-06"
_REPLACEMENTS = { "get_dv": "`waterdata.get_daily()`", ... }

@_deprecated
def get_dv(...):
    ...

Removal date

Set at 2027-05-06 (one full year), vs. the six months used for nadp in #243. nwis is the much more widely used legacy module and most users will need migration time. Easy to adjust if a different date fits the maintainer's plan better.

Test plan

  • 10 parametrized tests in tests/nwis_test.py::TestDeprecationWarnings pin the warning text — that the function name, the replacement helper, and the removal date all appear in the message.
  • An end-to-end test that calling get_iv() actually fires its DeprecationWarning (not just the standalone helper).
  • A nested-call test that pins the outermost-only contract for get_record(service="iv").
  • A 7-case parametrized tripwire that imports dataretrieval.waterdata and asserts every concrete callable named in _REPLACEMENTS exists — added in response to Copilot review (Deprecate remaining active nwis functions ahead of 2027-05-06 removal #271 review comment 3198266077). Prevents the PR from merging before its waterdata.* dependencies land.
  • All 47 tests/nwis_test.py tests pass.
  • ruff check clean.

Related

@thodson-usgs thodson-usgs requested a review from Copilot May 7, 2026 00:13
Copilot AI reviewed May 7, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the existing dataretrieval.nwis deprecation strategy by adding per-function DeprecationWarnings that include a concrete migration target in dataretrieval.waterdata, plus a scheduled removal date, and adds tests/NEWS coverage to pin the messaging.

Changes:

  • Added a _deprecated decorator + _warn_deprecated() helper in dataretrieval/nwis.py to emit per-function DeprecationWarnings (with re-entrancy suppression for wrapper call chains like get_record -> get_iv -> query_waterservices).
  • Added new tests in tests/nwis_test.py to assert warning text contents and ensure only one warning is emitted for nested calls.
  • Added a NEWS entry documenting the deprecation warnings and the planned NWIS removal date.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
dataretrieval/nwis.py Adds per-function deprecation warning infrastructure and decorates active NWIS entrypoints.
tests/nwis_test.py Adds parametrized tests that pin warning text + an end-to-end warning emission test and nested-call suppression test.
NEWS.md Documents the new per-function warnings and the planned removal date.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread dataretrieval/nwis.py
thodson-usgs added a commit to thodson-usgs/dataretrieval-python that referenced this pull request May 7, 2026
@thodson-usgs @claude
Add tripwire: every named waterdata replacement must exist
b09fbd2 
Addresses Copilot review on DOI-USGS#271: `_REPLACEMENTS` points
`get_discharge_peaks` -> `waterdata.get_peaks()` and `get_ratings` ->
`waterdata.get_ratings()`. If this PR merged before its dependencies
landed, users following the migration guidance would hit AttributeError.

Adds a parametrized test that imports `dataretrieval.waterdata` and
asserts every concrete callable named in the deprecation messages exists.
The test currently fails on `get_peaks` (still on PR DOI-USGS#267, not yet on
main) — exactly the desired behavior: this PR cannot merge before DOI-USGS#267.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
thodson-usgs and others added 4 commits May 6, 2026 19:24
@thodson-usgs @claude
Deprecate the remaining active nwis functions ahead of 2027-05-06 rem…
6510ff0 
…oval

The module-level "use waterdata instead" warning has been firing on
import for a while; this PR makes the migration guidance actionable
by emitting a per-function DeprecationWarning that names the specific
waterdata replacement the user should switch to.

Once peaks (DOI-USGS#267) and ratings (DOI-USGS#269) land, every active nwis function
has a waterdata replacement, so all nine of them are deprecated here:

  nwis.get_dv               -> waterdata.get_daily()
  nwis.get_iv               -> waterdata.get_continuous()
  nwis.get_info             -> waterdata.get_monitoring_locations()
  nwis.what_sites           -> waterdata.get_monitoring_locations()
  nwis.get_stats            -> waterdata.get_stats_por() /
                              waterdata.get_stats_date_range()
  nwis.get_discharge_peaks  -> waterdata.get_peaks()
  nwis.get_ratings          -> waterdata.get_ratings()
  nwis.get_record           -> the appropriate waterdata.get_*()
  nwis.query_waterdata      -> a high-level waterdata.get_*() helper
  nwis.query_waterservices  -> a high-level waterdata.get_*() helper

(get_qwdata, get_discharge_measurements, get_gwlevels, get_pmcodes,
and get_water_use are already defunct and raise NameError.)

Implementation follows the nadp deprecation template (DOI-USGS#243): a small
_REPLACEMENTS dict + a _warn_deprecated(func_name) helper called as
the first line of each public function. stacklevel=3 makes the
warning point at the caller's code, not the helper's frame.

11 new parametrized tests pin the warning text — that the function
name appears, the replacement helper appears, and the removal date
appears — plus one end-to-end test that get_iv() actually fires
its warning when called.

Removal date is set to 2027-05-06, one full year out (vs. the six
months used for nadp), since nwis is much more widely used and most
users will need migration time. Maintainer can adjust if desired.

This depends on DOI-USGS#267 (waterdata.get_peaks) and DOI-USGS#269 (waterdata.get_ratings)
being merged: until then the deprecation messages for get_discharge_peaks
and get_ratings point at functions users can't yet call. Hold this PR
draft until those land.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@thodson-usgs @claude
Suppress nested deprecation warnings; pin removal date via constant
c00ab77 
`get_record(service='iv')` calls `get_iv`, which calls `query_waterservices`
— each of which now emits a DeprecationWarning. Without suppression the user
sees three near-identical messages for one call.

Walk the call stack inside `_warn_deprecated`: if any ancestor frame is also
a deprecated nwis function, skip. Only the outermost call surfaces a
warning, regardless of how the wrappers are layered. Add a regression test
that pins this contract.

Loosen `test_warn_message_includes_replacement` to assert against the
`_NWIS_REMOVAL_DATE` constant rather than the literal "2027-05-06" so a
future date change updates one place, not two.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@thodson-usgs @claude
Switch to @_deprecated decorator; drop sys._getframe walk
5ca35c6 
Replaces nine `_warn_deprecated("name")` first-line calls with `@_deprecated`
above the function definition. The decorator does three things:

  1. Validates `func.__name__ in _REPLACEMENTS` at import time, so a missing
     mapping fails loudly instead of producing a KeyError on the first user
     call.
  2. Drops the stringly-typed `_warn_deprecated("get_iv")` call sites — the
     name now flows from `func.__name__`, so a typo can't drift from the
     real symbol.
  3. Replaces the `sys._getframe` stack walk with a thread-local re-entrancy
     flag. Because the decorator wraps the whole function call (not just the
     warn line), the flag's lifetime spans nested wrapper invocations
     correctly. No CPython implementation detail required.

Tighten the NEWS.md entry and drop the unused `requests_mock` fixture from
`test_warn_message_includes_replacement` (it never made an HTTP request).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@thodson-usgs @claude
Add tripwire: every named waterdata replacement must exist
78374f5 
Addresses Copilot review on DOI-USGS#271: `_REPLACEMENTS` points
`get_discharge_peaks` -> `waterdata.get_peaks()` and `get_ratings` ->
`waterdata.get_ratings()`. If this PR merged before its dependencies
landed, users following the migration guidance would hit AttributeError.

Adds a parametrized test that imports `dataretrieval.waterdata` and
asserts every concrete callable named in the deprecation messages exists.
The test currently fails on `get_peaks` (still on PR DOI-USGS#267, not yet on
main) — exactly the desired behavior: this PR cannot merge before DOI-USGS#267.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@thodson-usgs thodson-usgs marked this pull request as ready for review May 7, 2026 00:26
@thodson-usgs thodson-usgs requested a review from Copilot May 7, 2026 00:27
Copilot AI reviewed May 7, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread dataretrieval/nwis.py
@thodson-usgs thodson-usgs merged commit 18f831f into DOI-USGS:main May 7, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@thodson-usgs