Skip to content

refactor(structure): group src/hapi into layered subpackages #221

Description

@MAfarrag

Context

src/hapi holds 24 modules and 13,170 lines, and every one but four sits directly in the package root. The two
existing groupings are rrm/ and parameters/.

The flat root is the visible problem. rrm/ is the worse one: "rainfall-runoff model" currently spans three
different layers — the four conceptual models and their ABC, the distributed engine (distrrm.py), and the
calibration parameter distributor (parameters.py) — 3,883 lines under one name that describes only the first.

Problem / Current Behaviour

Three concrete costs:

  1. No layering is visible. hapi.protocols exists so the run layer never imports Catchment; that decoupling
    (PR refactor: split the builder from the run layer in Catchment/Run/Calibration #217) is recorded only as prose in a docstring. Nothing stops the next change reintroducing the edge.
  2. rrm/ misleads. A reader looking for the HBV models finds the spatial engine and the optimiser's parameter
    mapper next to them.
  3. Two unrelated parameters modules colliderrm/parameters.py maps the optimiser's flat vector onto the
    3D per-cell array; parameters/parameters.py downloads Beck et al. (2016) sets from FigShare. Nothing but
    the name connects them, and telling them apart currently takes prior knowledge.

Affected locations

File Lines What it actually is
src/hapi/inputs.py 2199 four independent classes plus the raster-reading helpers
src/hapi/catchment.py 1453 Catchment and Lake
src/hapi/rrm/parameters.py 875 spatial parameter distribution
src/hapi/parameters/parameters.py 654 FigShare download
src/hapi/rrm/distrrm.py 311 the per-cell loop and spatial routing — not a conceptual model
src/hapi/runs.py vs run.py 306 / 392 validated models vs entry points; the names are a coin flip

Motivation

The grouping below is derived from the measured runtime import graph (module-scope imports only; TYPE_CHECKING
edges excluded, since they do not constrain layout), not from a taxonomy. Fan-in, highest first:

6 results   5 runs   5 inputs   5 rrm.base_model   4 conceptual   4 period   3 protocols

The most-imported modules are not the biggest ones — they are the shared vocabulary. Any grouping that scatters
them produces upward imports.

Proposed Solution

hapi/
├── core/           period.py  config.py  silence.py
├── inputs/         rasters.py  meteo.py  network.py  geometry.py  preparation.py  dem.py
├── conceptual/     base.py  setup.py  hbv.py  hbv_bergestrom92.py  hbv_lake.py
├── simulation/     validated.py  results.py  protocols.py
├── model/          catchment.py  lake.py
├── engine/         run.py  wrapper.py  distributed.py  routing.py
├── calibration/    search.py  distribution.py
└── data/           figshare.py

Renames that remove real ambiguity: runs.pysimulation/validated.py, rrm/distrrm.py
engine/distributed.py, rrm/parameters.pycalibration/distribution.py, parameters/parameters.py
data/figshare.py, hapi_warnings.pycore/silence.py, conceptual.pyconceptual/setup.py.

Rank order — a module may import its own package or any lower rank:

rank 0  core, data          rank 2  simulation      rank 4  calibration
rank 1  inputs, conceptual  rank 3  model, engine

Checked mechanically against every module-scope edge: 0 upward, 0 sideways, 0 unplaced. The property this buys
is that model/ and engine/ are the same rank and never import each other — the #217 split, now enforceable as a
test or import-linter contract rather than a docstring.

Blast radius

hapi/__init__.py re-exports nothing but __version__, so the module path is the public API and every move is
breaking. Reference counts across src, tests, docs, examples:

58 hapi.catchment   42 hapi.inputs   42 hapi.rrm.hbv_bergestrom92   37 hapi.run   26 hapi.routing
18 hapi.calibration   16 hapi.conceptual   15 hapi.results   14 hapi.runs   12 hapi.rrm.parameters

Plus 18 docs/api/*.md mkdocstrings paths, two [[tool.mypy.overrides]] module lists, and three
[project.scripts] entry points. No packaging change needed —
[tool.setuptools.packages.find] include = ["hapi", "hapi.*"] already covers new subpackages.

Suggested sequencing

Fewest references first, one git mv + import rewrite per commit, suite green at each step:
datacoreconceptualsimulationenginecalibrationmodel. Then, as separate commits, the
two splits that need judgement rather than a move: inputs.py into five files, catchment.py into
catchment.py + lake.py.

Out of Scope

  • Any behaviour change. Moves, renames and import rewrites only.
  • A permanent home for dem.py. It is slated to migrate to digital-rivers (DEM.hand() supersedes
    hru_hand; convert_flow_direction_to_cell_indices supersedes flow_direction_index). inputs/dem.py is a way
    station, not a design decision.
  • Splitting results.py's rendering into simulation/rendering.py — worth doing, separate issue.

Relationship to existing issues

Effort Estimate

Size: L
Rationale: mechanical but wide — roughly 150 referencing files across seven move commits, plus docs, mypy and
entry-point updates. The two file splits can be deferred to keep it inside L.

Definition of Done

  • Compatibility stance decided: break the import paths outright (house precedent — the CamelCase→snake_case
    rename shipped with no shim), or start re-exporting the public names from hapi/__init__.py
  • The eight subpackages exist and rrm/ is gone
  • A test or import-linter contract asserts the rank order, and specifically that model/ never imports engine/
  • docs/api/*.md, the mypy overrides and the three console scripts all updated; mkdocs build --strict clean
  • The architecture docs are rewritten, and the note distinguishing the two parameters modules is no
    longer needed
  • Full suite, mypy and ruff green at every commit, not just at the end

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions