Background
dataretrieval.streamstats.get_watershed currently has a format parameter that selects between three return types:
format="geojson" (default): the raw requests.Responseformat="object": the parsed JSON dictformat="watershed": a Watershed instance built from the parsed JSON- anything else:
ValueError
The multi-return contract is documented in #245 (after that PR's fix). But the function's name and StreamStats' own conceptual model both suggest it should return a watershed.
Proposal
Drop the format parameter and have get_watershed always return a Watershed.
Pros:
- Single return type → cleaner type annotation (
-> Watershed), no Union[Response, dict, Watershed].
- Function name matches its contract.
- Removes the silent-fallthrough bug class entirely (no
format string to typo).
- Callers wanting the raw
Response can use requests.get(...); callers wanting JSON can call Watershed.from_streamstats_json(r.json()) or read Watershed.parameters / Watershed.workspace_id directly.
Cons:
- Breaking change for callers using
format="geojson" (the current default) or format="object".
- The default behavior changes silently if a caller passed no
format — they previously got a Response and would now get a Watershed.
Implementation sketch
Extract the HTTP+parse into a private helper (_fetch_streamstats_json) so get_watershed and Watershed.__init__ can share fetch logic without circular calls. A working draft of this lived briefly on the fix/streamstats-watershed-class branch (commit b3a5d29, since reverted to keep #245 non-breaking).
Decision needed
Is this worth the breaking change? If so, when — current minor cycle, next major, or behind a deprecation warning?
Background
dataretrieval.streamstats.get_watershedcurrently has aformatparameter that selects between three return types:format="geojson"(default): the rawrequests.Responseformat="object": the parsed JSONdictformat="watershed": aWatershedinstance built from the parsed JSONValueErrorThe multi-return contract is documented in #245 (after that PR's fix). But the function's name and StreamStats' own conceptual model both suggest it should return a watershed.
Proposal
Drop the
formatparameter and haveget_watershedalways return aWatershed.Pros:
-> Watershed), noUnion[Response, dict, Watershed].formatstring to typo).Responsecan userequests.get(...); callers wanting JSON can callWatershed.from_streamstats_json(r.json())or readWatershed.parameters/Watershed.workspace_iddirectly.Cons:
format="geojson"(the current default) orformat="object".format— they previously got aResponseand would now get aWatershed.Implementation sketch
Extract the HTTP+parse into a private helper (
_fetch_streamstats_json) soget_watershedandWatershed.__init__can share fetch logic without circular calls. A working draft of this lived briefly on thefix/streamstats-watershed-classbranch (commitb3a5d29, since reverted to keep #245 non-breaking).Decision needed
Is this worth the breaking change? If so, when — current minor cycle, next major, or behind a deprecation warning?