Polish wqp.py: silence prints, fix exception shape, fix typos by thodson-usgs · Pull Request #256 · DOI-USGS/dataretrieval-python

thodson-usgs · 2026-05-03T23:45:14Z

Summary

A handful of small, discrete polish fixes in dataretrieval/wqp.py:

Silence stdout: Six WQP helpers (what_organizations, what_projects, what_detection_limits, what_habitat_metrics, what_project_weights, what_activity_metrics) had print("WQX3.0 profile not available, returning legacy profile.") statements that fire whenever legacy=False is passed. Library code shouldn't print to stdout; converted to a single _warn_wqx3_unavailable() helper (alongside the existing _warn_wqx3_use() / _warn_legacy_use() helpers) so callers can filter, redirect, or capture them via warnings/pytest.warns. Also collapses the redundant if legacy is True: url = X else: warn(); url = X branches (both arms assigned the same legacy URL). The new _legacy_only_url helper additionally suppresses the legacy DeprecationWarning that wqp_url would otherwise raise — its message claims setting legacy=False removes it, which is a lie for endpoints that have no WQX3.0 alternative.
Fix exception shape in wqp_url / wqx3_url: TypeError("... Valid options are", f"{services}.") produced an exception whose args is a 2-tuple of strings, breaking the message rendering and using the wrong exception class (ValueError is the right type for an unrecognized argument value). Replaced with a single ValueError carrying one f-string.
Consistency: what_activity_metrics was the only read_csv site in this module without low_memory=False. Added it for parity with the other eight helpers.
Typos: Retrun -> Return (docstring); intermitttently -> intermittently (warning text).

Test plan

Existing 12 wqp tests still pass. (No new tests — each fix here is small enough that the diff is self-documenting and a regression would be loud at the API surface.)

Related PRs

Other open PRs in this bug-review series that touch dataretrieval/wqp.py (different functions, no functional conflicts):

Fix WQP_Metadata.site_info: bind as a real property and forward kwargs #249 — fix WQP_Metadata.site_info property binding.
Fix get_results: preserve user-supplied WQX3.0 dataProfile #250 — preserve user-supplied WQX3.0 dataProfile in get_results. An earlier draft of this PR also touched get_results's TypeError -> ValueError shape; that overlap was reverted so Fix get_results: preserve user-supplied WQX3.0 dataProfile #250 fully owns the get_results rewrite.

🤖 Generated with Claude Code

- Replace 6 stdout `print(...)` calls in `what_organizations`, `what_projects`, `what_detection_limits`, `what_habitat_metrics`, `what_project_weights`, and `what_activity_metrics` with `warnings.warn(..., UserWarning)` so callers can capture or filter the message instead of getting unconditional stdout pollution. - Convert `wqp_url`/`wqx3_url` from `TypeError(msg, msg)` (which raises with `args=(msg1, msg2)` and is the wrong type for an unrecognized argument) to `ValueError(single_msg)`. - Add `low_memory=False` to `what_activity_metrics` `read_csv` for consistency with the other 8 helpers. - Fix typos: `Retrun` -> `Return`, `intermitttently` -> `intermittently`. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

- Extract `_warn_wqx3_unavailable()` next to `_warn_wqx3_use()` and `_warn_legacy_use()`. Replaces 6 copies of an identical 4-line `warnings.warn(...)` block with a single helper call. `stacklevel=3` preserves the original `stacklevel=2` semantics through the extra helper frame. - Collapse `if legacy is True: url = ... else: warn(); url = ...` branches in 6 helpers — both arms assigned the same legacy URL — to `if not legacy: warn()` followed by a single assignment. - Apply the same `TypeError(msg, msg)` -> `ValueError(single_msg)` fix to `get_results`'s two `dataProfile` validators (the same broken pattern this PR already fixed in `wqp_url`/`wqx3_url`); also fixes the missing space in the joined "WQX3.0profile" message. - Add 2 regression tests for the `get_results` error paths. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

The simplify pass added `TypeError(msg, msg)` -> `ValueError(single_msg)` fixes to `get_results`'s two `dataProfile` validators. PR DOI-USGS#250 ("Fix get_results: preserve user-supplied WQX3.0 dataProfile") restructures the same code more thoroughly and addresses the same exception-shape bug as a side effect, so this PR should not duplicate that work. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Copilot

Pull request overview

This PR polishes the WQP helper module by replacing stdout print side effects with warnings, correcting exception types/messages for invalid service names, and making minor consistency/typo fixes.

Changes:

Replaced print(...) statements in several what_* helpers with a centralized _warn_wqx3_unavailable() warning helper and simplified redundant branching.
Fixed wqp_url / wqx3_url to raise a single-string ValueError (instead of tuple-arg TypeError) for unrecognized services.
Added low_memory=False to what_activity_metrics CSV parsing and corrected typos in docstrings/warning text.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`dataretrieval/wqp.py`	Replaces prints with warnings, fixes exception shape/type, aligns `read_csv` args, and corrects typos.
`tests/wqp_test.py`	Adds a regression test asserting `legacy=False` emits a warning for `what_organizations`.

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

+def _warn_wqx3_unavailable():
+    warnings.warn(
+        "WQX3.0 profile not available, returning legacy profile.",
+        UserWarning,
+        stacklevel=3,
+    )


+    with pytest.warns(UserWarning, match="WQX3.0 profile not available"):
+        what_organizations(
+            statecode="US:34", characteristicName="Chloride", legacy=False
+        )


Per copilot review on PR DOI-USGS#256: the six WQP helpers without WQX3.0 equivalents (what_organizations, what_projects, what_detection_limits, what_habitat_metrics, what_project_weights, what_activity_metrics) emit both _warn_wqx3_unavailable() (UserWarning) and _warn_legacy_use() (DeprecationWarning) when legacy=False is passed. The DeprecationWarning's text says 'Setting legacy=False will remove this warning' — a lie for endpoints that have no WQX3 alternative. Introduce a _legacy_only_url() helper that emits the WQX3-unavailable warning and suppresses the redundant legacy DeprecationWarning when legacy=False. Use it in all six helpers. Update the test to use catch_warnings(record=True) so it asserts both that the UserWarning fires AND that the misleading DeprecationWarning is suppressed. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

# Conflicts: # tests/wqp_test.py

Per /simplify review on PR DOI-USGS#256: - Unify the two `wqp_url(service)` returns in `_legacy_only_url` into a single return inside `warnings.catch_warnings()`, gating the warn and filter on `not legacy`. - Document the asymmetric `stacklevel=3` in `_warn_wqx3_unavailable`. - Hoist `import warnings` from inside the new regression test to the module-level imports in `tests/wqp_test.py`. - Compress the regression test's docstring. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

The legacy-only warning + DeprecationWarning suppression is small and the diff is self-documenting; a regression would be loud at the API surface (users would see the misleading "set legacy=False to remove this warning" message while already setting legacy=False). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

thodson-usgs and others added 3 commits May 3, 2026 18:44

This was referenced May 4, 2026

Fix WQP_Metadata.site_info: bind as a real property and forward kwargs #249

Draft

Fix get_results: preserve user-supplied WQX3.0 dataProfile #250

Merged

Drop trivial TypeError/ValueError tests for url helpers

e2c16fe

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

thodson-usgs requested a review from Copilot May 4, 2026 14:48

Copilot started reviewing on behalf of thodson-usgs May 4, 2026 14:49 View session

Copilot AI reviewed May 4, 2026

View reviewed changes

thodson-usgs and others added 4 commits May 4, 2026 10:14

Merge remote-tracking branch 'origin/main' into fix/wqp-polish

44794b4

# Conflicts: # tests/wqp_test.py

thodson-usgs marked this pull request as ready for review May 4, 2026 18:38

thodson-usgs merged commit 01c7fc7 into DOI-USGS:main May 4, 2026
8 checks passed

thodson-usgs deleted the fix/wqp-polish branch May 4, 2026 18:39

thodson-usgs mentioned this pull request May 6, 2026

Release staging: PRs targeted for v1.1.4 #270

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Polish wqp.py: silence prints, fix exception shape, fix typos#256

Polish wqp.py: silence prints, fix exception shape, fix typos#256
thodson-usgs merged 8 commits intoDOI-USGS:mainfrom
thodson-usgs:fix/wqp-polish

thodson-usgs commented May 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

thodson-usgs commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Related PRs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

thodson-usgs commented May 3, 2026 •

edited

Loading