Move the MIKE+ station lookup out of #702

Open
#706 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
35/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Active
Tech stack
python
Domain
databases

Research direction

Start by reading obs.py, tests/test_mikeplus.py, the relevant user-guide section, and ADR-013 alongside issue #702. Identify the MIKE+ lookup code and API surface that must be separated, then review the fixture, schema, path, naming, and locationtype concerns before choosing the branch point. Done means the lookup is isolated from the #702 move with its public API and tests documented for the remaining design decisions.

Written by the indexing model from the issue text.

Description

#702 carries a MIKE+ database lookup that places measured timeseries in the network:
_MikePlusStationResolver (~290 lines in obs.py), _observations_from_mikeplus, the db=
and source= arguments on NodeObservation.from_multiple and
ReachObservation.from_multiple, tests/test_mikeplus.py (558 lines), and a section of the
user guide. None of it is needed for the mikeio1d move #702 is about (ADR-013, Phase 2), and
it is not ready to ship on its own terms.

What needs more thought:

  • No fixture. Every test builds its own sqlite database with build_db, so the schema
    assumptions are checked only against our own construction of them. Nothing here has been
    read against a database MIKE+ wrote.
  • The locationtype codes are magic numbers. _NODE_TYPES = {8, 12},
    _LINK_TYPES = {9}. Any other code is dropped and named only on the failure path. No
    source says these are the codes, or that they are stable across MIKE+ versions.
  • resitemname is split on ; and the first field taken. An undocumented encoding.
  • tsfilename holds a Windows path and is matched on basename alone, case-folded. Two
    result files with the same name in different folders collide; source= is the only escape.
  • The nine columns across m_Station and m_Measurement are validated against one
    layout.
    The error says "The database layout is not the one modelskill expects" without
    recording which layout that is.
  • assetname becomes the observation name only when it is unique across the selection.
    The names a user gets depend on what else is in the database.

from_multiple(db=...) is public API, so whatever shape lands ships with 1.4.0.

Extract it onto its own branch, off #702 or off main once #702 lands.

Dominant language
Python
Stars
56
Forks
9
Avg merge
57m
Merged PRs (30d)
3

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from DHI/modelskill

All issues in DHI/modelskill

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.