From 99d759856ce26d5a6dd0005d6c5957d3a56f4d33 Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Sun, 3 May 2026 18:44:58 -0500 Subject: [PATCH 1/7] Polish wqp.py: silence prints, fix exception shape, fix typos - 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) --- dataretrieval/wqp.py | 52 +++++++++++++++++++++++++++++++------------- tests/wqp_test.py | 27 +++++++++++++++++++++++ 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/dataretrieval/wqp.py b/dataretrieval/wqp.py index 0b53e387..63b9a9c7 100644 --- a/dataretrieval/wqp.py +++ b/dataretrieval/wqp.py @@ -258,7 +258,11 @@ def what_organizations( if legacy is True: url = wqp_url("Organization") else: - print("WQX3.0 profile not available, returning legacy profile.") + warnings.warn( + "WQX3.0 profile not available, returning legacy profile.", + UserWarning, + stacklevel=2, + ) url = wqp_url("Organization") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -309,7 +313,11 @@ def what_projects(ssl_check=True, legacy=True, **kwargs): if legacy is True: url = wqp_url("Project") else: - print("WQX3.0 profile not available, returning legacy profile.") + warnings.warn( + "WQX3.0 profile not available, returning legacy profile.", + UserWarning, + stacklevel=2, + ) url = wqp_url("Project") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -435,7 +443,11 @@ def what_detection_limits( if legacy is True: url = wqp_url("ResultDetectionQuantitationLimit") else: - print("WQX3.0 profile not available, returning legacy profile.") + warnings.warn( + "WQX3.0 profile not available, returning legacy profile.", + UserWarning, + stacklevel=2, + ) url = wqp_url("ResultDetectionQuantitationLimit") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -490,7 +502,11 @@ def what_habitat_metrics( if legacy is True: url = wqp_url("BiologicalMetric") else: - print("WQX3.0 profile not available, returning legacy profile.") + warnings.warn( + "WQX3.0 profile not available, returning legacy profile.", + UserWarning, + stacklevel=2, + ) url = wqp_url("BiologicalMetric") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -516,7 +532,7 @@ def what_project_weights(ssl_check=True, legacy=True, **kwargs): ssl_check : bool Check the SSL certificate. Default is True. legacy : bool - Retrun the legacy WQX data profile. Default is True. + Return the legacy WQX data profile. Default is True. **kwargs : optional Accepts the same parameters as :obj:`dataretrieval.wqp.get_results` @@ -546,7 +562,11 @@ def what_project_weights(ssl_check=True, legacy=True, **kwargs): if legacy is True: url = wqp_url("ProjectMonitoringLocationWeighting") else: - print("WQX3.0 profile not available, returning legacy profile.") + warnings.warn( + "WQX3.0 profile not available, returning legacy profile.", + UserWarning, + stacklevel=2, + ) url = wqp_url("ProjectMonitoringLocationWeighting") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -602,12 +622,16 @@ def what_activity_metrics(ssl_check=True, legacy=True, **kwargs): if legacy is True: url = wqp_url("ActivityMetric") else: - print("WQX3.0 profile not available, returning legacy profile.") + warnings.warn( + "WQX3.0 profile not available, returning legacy profile.", + UserWarning, + stacklevel=2, + ) url = wqp_url("ActivityMetric") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) - df = pd.read_csv(StringIO(response.text), delimiter=",") + df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False) return df, WQP_Metadata(response) @@ -619,9 +643,8 @@ def wqp_url(service): _warn_legacy_use() if service not in services_legacy: - raise TypeError( - "Legacy service not recognized. Valid options are", - f"{services_legacy}.", + raise ValueError( + f"Legacy service not recognized. Valid options are {services_legacy}." ) return f"{base_url}{service}/Search?" @@ -634,9 +657,8 @@ def wqx3_url(service): _warn_wqx3_use() if service not in services_wqx3: - raise TypeError( - "WQX3.0 service not recognized. Valid options are", - f"{services_wqx3}.", + raise ValueError( + f"WQX3.0 service not recognized. Valid options are {services_wqx3}." ) return f"{base_url}{service}/search?" @@ -708,7 +730,7 @@ def _check_kwargs(kwargs): def _warn_wqx3_use(): message = ( "Support for the WQX3.0 profiles is experimental. " - "Queries may be slow or fail intermitttently." + "Queries may be slow or fail intermittently." ) warnings.warn(message, UserWarning, stacklevel=2) diff --git a/tests/wqp_test.py b/tests/wqp_test.py index f36558bc..8d31a5a4 100644 --- a/tests/wqp_test.py +++ b/tests/wqp_test.py @@ -14,6 +14,8 @@ what_project_weights, what_projects, what_sites, + wqp_url, + wqx3_url, ) @@ -216,3 +218,28 @@ def test_check_kwargs(): kwargs = {"mimeType": "foo"} with pytest.raises(ValueError): kwargs = _check_kwargs(kwargs) + + +def test_wqp_url_unknown_service_raises_value_error(): + """Unknown legacy service should raise ValueError with a single readable message.""" + with pytest.raises(ValueError, match="Legacy service not recognized"): + wqp_url("NotAService") + + +def test_wqx3_url_unknown_service_raises_value_error(): + """Unknown WQX3.0 service should raise ValueError with a single readable message.""" + with pytest.raises(ValueError, match="WQX3.0 service not recognized"): + wqx3_url("NotAService") + + +def test_what_organizations_legacy_false_warns(requests_mock): + """legacy=False on what_organizations should emit a warning, not print to stdout.""" + request_url = ( + "https://www.waterqualitydata.us/data/Organization/Search?statecode=US%3A34" + "&characteristicName=Chloride&mimeType=csv" + ) + mock_request(requests_mock, request_url, "tests/data/wqp_organizations.txt") + with pytest.warns(UserWarning, match="WQX3.0 profile not available"): + what_organizations( + statecode="US:34", characteristicName="Chloride", legacy=False + ) From 8c86ebd94759ea77922d511450664816d40f090f Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Sun, 3 May 2026 20:04:00 -0500 Subject: [PATCH 2/7] Apply simplify findings: extract _warn_wqx3_unavailable, fix get_results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- dataretrieval/wqp.py | 92 +++++++++++++++----------------------------- tests/wqp_test.py | 12 ++++++ 2 files changed, 44 insertions(+), 60 deletions(-) diff --git a/dataretrieval/wqp.py b/dataretrieval/wqp.py index 63b9a9c7..09ce7d4d 100644 --- a/dataretrieval/wqp.py +++ b/dataretrieval/wqp.py @@ -131,9 +131,9 @@ def get_results( "dataProfile" in kwargs and kwargs["dataProfile"] not in result_profiles_legacy ): - raise TypeError( - f"dataProfile {kwargs['dataProfile']} is not a legacy profile.", - f"Valid options are {result_profiles_legacy}.", + raise ValueError( + f"dataProfile {kwargs['dataProfile']} is not a legacy profile. " + f"Valid options are {result_profiles_legacy}." ) url = wqp_url("Result") @@ -143,9 +143,9 @@ def get_results( "dataProfile" in kwargs and kwargs["dataProfile"] not in result_profiles_wqx3 ): - raise TypeError( - f"dataProfile {kwargs['dataProfile']} is not a valid WQX3.0" - f"profile. Valid options are {result_profiles_wqx3}.", + raise ValueError( + f"dataProfile {kwargs['dataProfile']} is not a valid WQX3.0 " + f"profile. Valid options are {result_profiles_wqx3}." ) else: kwargs["dataProfile"] = "fullPhysChem" @@ -255,15 +255,9 @@ def what_organizations( kwargs = _check_kwargs(kwargs) - if legacy is True: - url = wqp_url("Organization") - else: - warnings.warn( - "WQX3.0 profile not available, returning legacy profile.", - UserWarning, - stacklevel=2, - ) - url = wqp_url("Organization") + if not legacy: + _warn_wqx3_unavailable() + url = wqp_url("Organization") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -310,15 +304,9 @@ def what_projects(ssl_check=True, legacy=True, **kwargs): kwargs = _check_kwargs(kwargs) - if legacy is True: - url = wqp_url("Project") - else: - warnings.warn( - "WQX3.0 profile not available, returning legacy profile.", - UserWarning, - stacklevel=2, - ) - url = wqp_url("Project") + if not legacy: + _warn_wqx3_unavailable() + url = wqp_url("Project") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -440,15 +428,9 @@ def what_detection_limits( kwargs = _check_kwargs(kwargs) - if legacy is True: - url = wqp_url("ResultDetectionQuantitationLimit") - else: - warnings.warn( - "WQX3.0 profile not available, returning legacy profile.", - UserWarning, - stacklevel=2, - ) - url = wqp_url("ResultDetectionQuantitationLimit") + if not legacy: + _warn_wqx3_unavailable() + url = wqp_url("ResultDetectionQuantitationLimit") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -499,15 +481,9 @@ def what_habitat_metrics( kwargs = _check_kwargs(kwargs) - if legacy is True: - url = wqp_url("BiologicalMetric") - else: - warnings.warn( - "WQX3.0 profile not available, returning legacy profile.", - UserWarning, - stacklevel=2, - ) - url = wqp_url("BiologicalMetric") + if not legacy: + _warn_wqx3_unavailable() + url = wqp_url("BiologicalMetric") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -559,15 +535,9 @@ def what_project_weights(ssl_check=True, legacy=True, **kwargs): kwargs = _check_kwargs(kwargs) - if legacy is True: - url = wqp_url("ProjectMonitoringLocationWeighting") - else: - warnings.warn( - "WQX3.0 profile not available, returning legacy profile.", - UserWarning, - stacklevel=2, - ) - url = wqp_url("ProjectMonitoringLocationWeighting") + if not legacy: + _warn_wqx3_unavailable() + url = wqp_url("ProjectMonitoringLocationWeighting") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -619,15 +589,9 @@ def what_activity_metrics(ssl_check=True, legacy=True, **kwargs): kwargs = _check_kwargs(kwargs) - if legacy is True: - url = wqp_url("ActivityMetric") - else: - warnings.warn( - "WQX3.0 profile not available, returning legacy profile.", - UserWarning, - stacklevel=2, - ) - url = wqp_url("ActivityMetric") + if not legacy: + _warn_wqx3_unavailable() + url = wqp_url("ActivityMetric") response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -744,3 +708,11 @@ def _warn_legacy_use(): "will remove this warning." ) warnings.warn(message, DeprecationWarning, stacklevel=2) + + +def _warn_wqx3_unavailable(): + warnings.warn( + "WQX3.0 profile not available, returning legacy profile.", + UserWarning, + stacklevel=3, + ) diff --git a/tests/wqp_test.py b/tests/wqp_test.py index 8d31a5a4..fd1747ff 100644 --- a/tests/wqp_test.py +++ b/tests/wqp_test.py @@ -243,3 +243,15 @@ def test_what_organizations_legacy_false_warns(requests_mock): what_organizations( statecode="US:34", characteristicName="Chloride", legacy=False ) + + +def test_get_results_invalid_legacy_dataprofile_raises_value_error(): + """Invalid legacy dataProfile -> ValueError with single readable message.""" + with pytest.raises(ValueError, match="is not a legacy profile"): + get_results(dataProfile="not_a_real_profile") + + +def test_get_results_invalid_wqx3_dataprofile_raises_value_error(): + """Invalid WQX3.0 dataProfile -> ValueError with single readable message.""" + with pytest.raises(ValueError, match="is not a valid WQX3.0 profile"): + get_results(legacy=False, dataProfile="not_a_real_profile") From e5d1afdf0e5664a192c0bd4957e6e660b42fcd20 Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Sun, 3 May 2026 20:23:07 -0500 Subject: [PATCH 3/7] Revert get_results dataProfile changes (covered by PR #250) The simplify pass added `TypeError(msg, msg)` -> `ValueError(single_msg)` fixes to `get_results`'s two `dataProfile` validators. PR #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) --- dataretrieval/wqp.py | 12 ++++++------ tests/wqp_test.py | 12 ------------ 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/dataretrieval/wqp.py b/dataretrieval/wqp.py index 09ce7d4d..d8d68c23 100644 --- a/dataretrieval/wqp.py +++ b/dataretrieval/wqp.py @@ -131,9 +131,9 @@ def get_results( "dataProfile" in kwargs and kwargs["dataProfile"] not in result_profiles_legacy ): - raise ValueError( - f"dataProfile {kwargs['dataProfile']} is not a legacy profile. " - f"Valid options are {result_profiles_legacy}." + raise TypeError( + f"dataProfile {kwargs['dataProfile']} is not a legacy profile.", + f"Valid options are {result_profiles_legacy}.", ) url = wqp_url("Result") @@ -143,9 +143,9 @@ def get_results( "dataProfile" in kwargs and kwargs["dataProfile"] not in result_profiles_wqx3 ): - raise ValueError( - f"dataProfile {kwargs['dataProfile']} is not a valid WQX3.0 " - f"profile. Valid options are {result_profiles_wqx3}." + raise TypeError( + f"dataProfile {kwargs['dataProfile']} is not a valid WQX3.0" + f"profile. Valid options are {result_profiles_wqx3}.", ) else: kwargs["dataProfile"] = "fullPhysChem" diff --git a/tests/wqp_test.py b/tests/wqp_test.py index fd1747ff..8d31a5a4 100644 --- a/tests/wqp_test.py +++ b/tests/wqp_test.py @@ -243,15 +243,3 @@ def test_what_organizations_legacy_false_warns(requests_mock): what_organizations( statecode="US:34", characteristicName="Chloride", legacy=False ) - - -def test_get_results_invalid_legacy_dataprofile_raises_value_error(): - """Invalid legacy dataProfile -> ValueError with single readable message.""" - with pytest.raises(ValueError, match="is not a legacy profile"): - get_results(dataProfile="not_a_real_profile") - - -def test_get_results_invalid_wqx3_dataprofile_raises_value_error(): - """Invalid WQX3.0 dataProfile -> ValueError with single readable message.""" - with pytest.raises(ValueError, match="is not a valid WQX3.0 profile"): - get_results(legacy=False, dataProfile="not_a_real_profile") From e2c16fe8fe00fb983160e866416c84e62059ab27 Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Mon, 4 May 2026 01:58:15 -0500 Subject: [PATCH 4/7] Drop trivial TypeError/ValueError tests for url helpers Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/wqp_test.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/wqp_test.py b/tests/wqp_test.py index 8d31a5a4..c7d966fb 100644 --- a/tests/wqp_test.py +++ b/tests/wqp_test.py @@ -14,8 +14,6 @@ what_project_weights, what_projects, what_sites, - wqp_url, - wqx3_url, ) @@ -220,18 +218,6 @@ def test_check_kwargs(): kwargs = _check_kwargs(kwargs) -def test_wqp_url_unknown_service_raises_value_error(): - """Unknown legacy service should raise ValueError with a single readable message.""" - with pytest.raises(ValueError, match="Legacy service not recognized"): - wqp_url("NotAService") - - -def test_wqx3_url_unknown_service_raises_value_error(): - """Unknown WQX3.0 service should raise ValueError with a single readable message.""" - with pytest.raises(ValueError, match="WQX3.0 service not recognized"): - wqx3_url("NotAService") - - def test_what_organizations_legacy_false_warns(requests_mock): """legacy=False on what_organizations should emit a warning, not print to stdout.""" request_url = ( From 199bc711298c0f818571d7f13bd2aa5349fda6ba Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Mon, 4 May 2026 10:14:16 -0500 Subject: [PATCH 5/7] Suppress redundant legacy DeprecationWarning when WQX3 unavailable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per copilot review on PR #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) --- dataretrieval/wqp.py | 41 +++++++++++++++++++++++------------------ tests/wqp_test.py | 19 +++++++++++++++++-- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/dataretrieval/wqp.py b/dataretrieval/wqp.py index d8d68c23..f6b18ba1 100644 --- a/dataretrieval/wqp.py +++ b/dataretrieval/wqp.py @@ -255,9 +255,7 @@ def what_organizations( kwargs = _check_kwargs(kwargs) - if not legacy: - _warn_wqx3_unavailable() - url = wqp_url("Organization") + url = _legacy_only_url("Organization", legacy=legacy) response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -304,9 +302,7 @@ def what_projects(ssl_check=True, legacy=True, **kwargs): kwargs = _check_kwargs(kwargs) - if not legacy: - _warn_wqx3_unavailable() - url = wqp_url("Project") + url = _legacy_only_url("Project", legacy=legacy) response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -428,9 +424,7 @@ def what_detection_limits( kwargs = _check_kwargs(kwargs) - if not legacy: - _warn_wqx3_unavailable() - url = wqp_url("ResultDetectionQuantitationLimit") + url = _legacy_only_url("ResultDetectionQuantitationLimit", legacy=legacy) response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -481,9 +475,7 @@ def what_habitat_metrics( kwargs = _check_kwargs(kwargs) - if not legacy: - _warn_wqx3_unavailable() - url = wqp_url("BiologicalMetric") + url = _legacy_only_url("BiologicalMetric", legacy=legacy) response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -535,9 +527,7 @@ def what_project_weights(ssl_check=True, legacy=True, **kwargs): kwargs = _check_kwargs(kwargs) - if not legacy: - _warn_wqx3_unavailable() - url = wqp_url("ProjectMonitoringLocationWeighting") + url = _legacy_only_url("ProjectMonitoringLocationWeighting", legacy=legacy) response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -589,9 +579,7 @@ def what_activity_metrics(ssl_check=True, legacy=True, **kwargs): kwargs = _check_kwargs(kwargs) - if not legacy: - _warn_wqx3_unavailable() - url = wqp_url("ActivityMetric") + url = _legacy_only_url("ActivityMetric", legacy=legacy) response = query(url, payload=kwargs, delimiter=";", ssl_check=ssl_check) @@ -716,3 +704,20 @@ def _warn_wqx3_unavailable(): UserWarning, stacklevel=3, ) + + +def _legacy_only_url(service: str, legacy: bool) -> str: + """URL builder for WQP services that have no WQX3.0 equivalent. + + When ``legacy=False`` is passed to one of these helpers we emit a + ``UserWarning`` explaining the fallback and *also* suppress the legacy + ``DeprecationWarning`` that ``wqp_url`` would otherwise raise — its + message claims setting ``legacy=False`` removes the warning, which is + a lie for endpoints that have no WQX3.0 alternative. + """ + if not legacy: + _warn_wqx3_unavailable() + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + return wqp_url(service) + return wqp_url(service) diff --git a/tests/wqp_test.py b/tests/wqp_test.py index c7d966fb..1c7318ac 100644 --- a/tests/wqp_test.py +++ b/tests/wqp_test.py @@ -219,13 +219,28 @@ def test_check_kwargs(): def test_what_organizations_legacy_false_warns(requests_mock): - """legacy=False on what_organizations should emit a warning, not print to stdout.""" + """legacy=False on a legacy-only helper: + - emits the WQX3.0-unavailable UserWarning + - suppresses the redundant legacy DeprecationWarning, whose text would + misleadingly tell the user that setting legacy=False removes it. + """ + import warnings + request_url = ( "https://www.waterqualitydata.us/data/Organization/Search?statecode=US%3A34" "&characteristicName=Chloride&mimeType=csv" ) mock_request(requests_mock, request_url, "tests/data/wqp_organizations.txt") - with pytest.warns(UserWarning, match="WQX3.0 profile not available"): + + with warnings.catch_warnings(record=True) as captured: + warnings.simplefilter("always") what_organizations( statecode="US:34", characteristicName="Chloride", legacy=False ) + + user_warnings = [w for w in captured if issubclass(w.category, UserWarning)] + deprecation_warnings = [ + w for w in captured if issubclass(w.category, DeprecationWarning) + ] + assert any("WQX3.0 profile not available" in str(w.message) for w in user_warnings) + assert not any("legacy WQX format" in str(w.message) for w in deprecation_warnings) From 8f2de13b1757e6c5c52ded191acb0efb245c9b1f Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Mon, 4 May 2026 12:12:19 -0500 Subject: [PATCH 6/7] Tighten _legacy_only_url and clean up test imports Per /simplify review on PR #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) --- dataretrieval/wqp.py | 10 +++++----- tests/wqp_test.py | 10 +++------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/dataretrieval/wqp.py b/dataretrieval/wqp.py index 601deca8..24e1737e 100644 --- a/dataretrieval/wqp.py +++ b/dataretrieval/wqp.py @@ -691,6 +691,7 @@ def _warn_legacy_use(): def _warn_wqx3_unavailable(): + # stacklevel=3: warn -> _warn_wqx3_unavailable -> _legacy_only_url -> what_* warnings.warn( "WQX3.0 profile not available, returning legacy profile.", UserWarning, @@ -707,9 +708,8 @@ def _legacy_only_url(service: str, legacy: bool) -> str: message claims setting ``legacy=False`` removes the warning, which is a lie for endpoints that have no WQX3.0 alternative. """ - if not legacy: - _warn_wqx3_unavailable() - with warnings.catch_warnings(): + with warnings.catch_warnings(): + if not legacy: + _warn_wqx3_unavailable() warnings.simplefilter("ignore", DeprecationWarning) - return wqp_url(service) - return wqp_url(service) + return wqp_url(service) diff --git a/tests/wqp_test.py b/tests/wqp_test.py index a2d299ec..1c579860 100644 --- a/tests/wqp_test.py +++ b/tests/wqp_test.py @@ -1,4 +1,5 @@ import datetime +import warnings import pytest from pandas import DataFrame @@ -219,13 +220,8 @@ def test_check_kwargs(): def test_what_organizations_legacy_false_warns(requests_mock): - """legacy=False on a legacy-only helper: - - emits the WQX3.0-unavailable UserWarning - - suppresses the redundant legacy DeprecationWarning, whose text would - misleadingly tell the user that setting legacy=False removes it. - """ - import warnings - + """legacy=False on a legacy-only helper warns and suppresses the + misleading legacy DeprecationWarning.""" request_url = ( "https://www.waterqualitydata.us/data/Organization/Search?statecode=US%3A34" "&characteristicName=Chloride&mimeType=csv" From 4f1c6b191f49105c3c3ec9d480acb517837a952b Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Mon, 4 May 2026 13:36:58 -0500 Subject: [PATCH 7/7] Drop trivial regression test from wqp polish PR 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) --- tests/wqp_test.py | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/tests/wqp_test.py b/tests/wqp_test.py index 1c579860..a337f7ec 100644 --- a/tests/wqp_test.py +++ b/tests/wqp_test.py @@ -1,5 +1,4 @@ import datetime -import warnings import pytest from pandas import DataFrame @@ -219,29 +218,6 @@ def test_check_kwargs(): kwargs = _check_kwargs(kwargs) -def test_what_organizations_legacy_false_warns(requests_mock): - """legacy=False on a legacy-only helper warns and suppresses the - misleading legacy DeprecationWarning.""" - request_url = ( - "https://www.waterqualitydata.us/data/Organization/Search?statecode=US%3A34" - "&characteristicName=Chloride&mimeType=csv" - ) - mock_request(requests_mock, request_url, "tests/data/wqp_organizations.txt") - - with warnings.catch_warnings(record=True) as captured: - warnings.simplefilter("always") - what_organizations( - statecode="US:34", characteristicName="Chloride", legacy=False - ) - - user_warnings = [w for w in captured if issubclass(w.category, UserWarning)] - deprecation_warnings = [ - w for w in captured if issubclass(w.category, DeprecationWarning) - ] - assert any("WQX3.0 profile not available" in str(w.message) for w in user_warnings) - assert not any("legacy WQX format" in str(w.message) for w in deprecation_warnings) - - def test_get_results_wqx3_preserves_user_dataProfile(requests_mock): """A valid user-supplied WQX3.0 profile must not be overwritten.