Fix Watershed class: persist instance state and parse JSON

[thodson-usgs]

Summary

• Watershed.from_streamstats_json was a classmethod that mutated cls instead of building an instance, causing every watershed to share class-level state and the method to return the class itself rather than a Watershed.
• Watershed.__init__(rcode, x, y) called get_watershed(...) and discarded the result, so a freshly-constructed instance had no attributes of its own.
• get_watershed(format="object") returned None instead of the parsed JSON dict its docstring promised.

This PR makes from_streamstats_json return a real instance, makes __init__ populate self via get_watershed(format="object"), and makes format="object" actually return the parsed JSON. The legacy _workspaceID attribute is preserved as a read-only alias for the new workspace_id.

While here:

• Dropped the dead format == "shape" branch (a stale Fiona-stub pass that fell through to a Watershed despite documenting a different return type).
• Made the format contract explicit: added a "watershed" case alongside "geojson"/"object", and unrecognized values now raise ValueError. Previously any unrecognized format silently fell through to a Watershed — the same bug shape that masked the broken "shape" branch.
• Fixed get_sample_watershed to actually return a Watershed instance (preexisting docstring/behavior mismatch — it called get_watershed(...) with the default format="geojson" and so returned the raw Response).
• Switched json.loads(r.text) to r.json() to match the convention used in nldi.py/nwis.py.
• Rewrote the get_watershed narrative docstring to describe the multi-return contract (Response / dict / Watershed selected by format) instead of the previous "always returns a watershed" wording, which contradicted the actual default behavior.

Minimal reproducible examples

Reconstructed inline because the live StreamStats endpoint is currently returning 404 across requests; the bug shapes are pure-Python and don't depend on the API.

Bug 1: from_streamstats_json mutates the class

class BuggyWatershed:
    @classmethod
    def from_streamstats_json(cls, blob):
        cls.workspace_id = blob["workspaceID"]   # mutates the class
        cls.parameters = blob["parameters"]
        return cls                                # returns the class itself

a = BuggyWatershed.from_streamstats_json({"workspaceID": "WS_A", "parameters": ["a"]})
b = BuggyWatershed.from_streamstats_json({"workspaceID": "WS_B", "parameters": ["b"]})
print(a is BuggyWatershed)                       # True — not an instance!
print(a.workspace_id, b.workspace_id)            # WS_B WS_B — shared class-level state

Bug 2: __init__ discards the work

class BuggyWatershed:
    def __init__(self, rcode, x, y):
        _ = get_watershed(rcode, x, y)            # result discarded

ws = BuggyWatershed("MA", -72.7, 42.6)
print(hasattr(ws, "workspace_id"))                # False

Bug 3: format="object" returns None

def buggy_get_watershed(rcode, x, y, format="geojson"):
    if format == "geojson":
        return _response
    elif format == "object":
        pass                                      # implicit None

After this PR, format="object" returns the parsed dict, Watershed.from_streamstats_json returns a fresh instance with isolated state, and Watershed(...) populates self.

Test plan

• One regression test in tests/streamstats_test.py covering the load-bearing behavior change: two watersheds built from different JSON must not share state via class-level attributes (test_from_streamstats_json_does_not_mutate_class). The other behaviors — from_streamstats_json returns an instance, format="object" returns a dict, __init__ populates self, _workspaceID resolves — are each one-line obvious fixes; their tests were dropped as trivial.
• Full local test suite passes.
• Reviewer: live-API verification was attempted but the StreamStats watershed.geojson endpoint is currently returning HTTP 404 for all tested coordinates (service-side issue independent of this PR). Suggested follow-up: re-run the existing test_get_watershed integration once StreamStats recovers.

Follow-up

Filed a separate issue to discuss whether get_watershed should always return a Watershed (drop the format parameter). That would match the function name and the original narrative-docstring promise, but is a breaking change and out of scope for this bug-fix PR.

[Copilot]

Pull request overview

Fixes dataretrieval.streamstats.Watershed construction/parsing so instances hold their own state and get_watershed(format="object") returns parsed JSON as advertised, with new regression tests to prevent class-level state leaks.

Changes:

• Make get_watershed(format="object") return a parsed JSON dict (instead of None) and reuse the parsed data for Watershed creation.
• Rework Watershed.from_streamstats_json to construct and populate a real instance (no class-level mutation), and make Watershed.__init__ populate instance attributes.
• Add tests/streamstats_test.py covering the regressions and the _workspaceID back-compat alias.

Reviewed changes

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

File	Description
dataretrieval/streamstats.py	Fixes JSON parsing/return behavior and corrects Watershed instance construction/state population.
tests/streamstats_test.py	Adds regression tests for instance state, object-format return, init population, and _workspaceID aliasing.

---

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

[Copilot]

Pull request overview

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

---

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