Skip to content

Commit 8fdd267

Browse files
thodson-usgsclaude
andcommitted
Fix get_results: preserve user-supplied WQX3.0 dataProfile
The validation block silently overwrote any user-supplied WQX3.0 dataProfile because the `else` clause attached to the *outer* `if "dataProfile" in kwargs and ... not in result_profiles_wqx3` and fired whenever that compound condition was False — including the case where the user passed a *valid* profile like "narrow" or "basicPhysChem". The documented example `dataProfile="narrow"` therefore did not actually request the narrow profile. Restructure the WQX3.0 branch so the default (`fullPhysChem`) is only applied when the user did *not* set `dataProfile` at all. Also raise `ValueError` rather than `TypeError` for invalid profile values (these are bad values, not bad types) and call the constructor with a single formatted message — the previous code passed two separate strings, producing a tuple-args exception. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 51ac674 commit 8fdd267

2 files changed

Lines changed: 43 additions & 11 deletions

File tree

dataretrieval/wqp.py

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -131,22 +131,20 @@ def get_results(
131131
"dataProfile" in kwargs
132132
and kwargs["dataProfile"] not in result_profiles_legacy
133133
):
134-
raise TypeError(
135-
f"dataProfile {kwargs['dataProfile']} is not a legacy profile.",
136-
f"Valid options are {result_profiles_legacy}.",
134+
raise ValueError(
135+
f"dataProfile {kwargs['dataProfile']!r} is not a legacy profile. "
136+
f"Valid options are {result_profiles_legacy}."
137137
)
138138

139139
url = wqp_url("Result")
140140

141141
else:
142-
if (
143-
"dataProfile" in kwargs
144-
and kwargs["dataProfile"] not in result_profiles_wqx3
145-
):
146-
raise TypeError(
147-
f"dataProfile {kwargs['dataProfile']} is not a valid WQX3.0"
148-
f"profile. Valid options are {result_profiles_wqx3}.",
149-
)
142+
if "dataProfile" in kwargs:
143+
if kwargs["dataProfile"] not in result_profiles_wqx3:
144+
raise ValueError(
145+
f"dataProfile {kwargs['dataProfile']!r} is not a valid "
146+
f"WQX3.0 profile. Valid options are {result_profiles_wqx3}."
147+
)
150148
else:
151149
kwargs["dataProfile"] = "fullPhysChem"
152150

tests/wqp_test.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,37 @@ def test_check_kwargs():
216216
kwargs = {"mimeType": "foo"}
217217
with pytest.raises(ValueError):
218218
kwargs = _check_kwargs(kwargs)
219+
220+
221+
def test_get_results_wqx3_preserves_user_dataProfile(requests_mock):
222+
"""A valid user-supplied WQX3.0 profile must not be overwritten.
223+
224+
Regression: previously the `else` branch of the `dataProfile` validation
225+
triggered whenever the value was *not invalid*, including any valid
226+
user-supplied profile, silently overwriting it with 'fullPhysChem'.
227+
"""
228+
request_url = (
229+
"https://www.waterqualitydata.us/wqx3/Result/search?"
230+
"siteid=UTAHDWQ_WQX-4993795&mimeType=csv&dataProfile=narrow"
231+
)
232+
response_file_path = "tests/data/wqp3_results.txt"
233+
mock_request(requests_mock, request_url, response_file_path)
234+
235+
df, _md = get_results(
236+
legacy=False, siteid="UTAHDWQ_WQX-4993795", dataProfile="narrow"
237+
)
238+
assert isinstance(df, DataFrame)
239+
sent = requests_mock.request_history[-1]
240+
assert sent.qs.get("dataprofile") == ["narrow"]
241+
242+
243+
def test_get_results_wqx3_invalid_dataProfile_raises():
244+
"""Invalid WQX3.0 dataProfile raises ValueError, not TypeError."""
245+
with pytest.raises(ValueError, match="WQX3.0 profile"):
246+
get_results(legacy=False, dataProfile="not_a_real_profile")
247+
248+
249+
def test_get_results_legacy_invalid_dataProfile_raises():
250+
"""Invalid legacy dataProfile raises ValueError, not TypeError."""
251+
with pytest.raises(ValueError, match="legacy profile"):
252+
get_results(legacy=True, dataProfile="not_a_real_profile")

0 commit comments

Comments
 (0)