Skip to content

refactor(catchment): decide what extract_discharge(only_outlet=) should do #209

Description

@MAfarrag

Context

Found during a review of extract_discharge. The parameter is public, documented as doing something, and has
never done it. Present on main; documented as inert rather than changed, because fixing it alters behaviour and
deserves its own decision.

Problem / Current Behaviour

Catchment.extract_discharge dispatches like this:

if not frame_work_1:
    ...                      # per-gauge extraction
elif frame_work_1 or only_outlet:
    ...                      # basin-wide sum

The elif is reached only when frame_work_1 is already True, so only_outlet can never change which branch
runs. extract_discharge(only_outlet=True) behaves exactly like extract_discharge().

The docstring says "True to extract discharge only at the outlet cell", so a caller has no way to discover that
the flag does nothing short of reading the dispatch.

Affected locations

File Symbol Notes
src/hapi/catchment.py Catchment.extract_discharge the elif frame_work_1 or only_outlet dispatch
src/hapi/calibration.py Calibration.extract_discharge override; accepts and ignores the same flag

Steps to Reproduce

model.extract_discharge(only_outlet=True)
# identical result to model.extract_discharge() -- the flag selects nothing

Proposed Solution

Decide between three, none of which is obviously right:

  1. Make it work — give only_outlet its own branch, independent of frame_work_1. Changes behaviour for
    anyone passing it today, though since it currently does nothing, nobody can be relying on it working.
  2. Remove it — drop the parameter from both signatures. Cleanest, but breaks any call site passing it by
    keyword.
  3. Keep documenting it as inert — the current state, chosen to avoid a behaviour change inside an unrelated
    refactor.

Whichever is chosen, the Calibration override should follow.

Out of Scope

  • The MAXBAS guard in the same method, which is separate and working.

Effort Estimate

Size: XS
Rationale: a few lines either way; the cost is the decision, not the change.

Definition of Done

  • A decision is recorded on the issue
  • only_outlet either selects the outlet-cell path, or is gone from both signatures
  • The docstrings match the behaviour
  • Tests cover whichever behaviour is chosen
  • All existing tests continue to pass

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingquestionFurther information is requested

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions