Skip to content

Commit a5dec82

Browse files
jpalm3rclaude
andcommitted
Reconcile companion node names against the main result file
mikeio1d decodes '.res' text as UTF-8 but '.resx' text as the Windows ANSI codepage, so a node named 'ØST' in one file is 'ØST' in the other. Four Danish tank names in a real EPANET model were enough to make the two files look like different models and fail validation. Worse when validation passed: the merge looks locations up by the main file's name, so those nodes lost their companion quantities silently. The repair is only used when it produces a name the main file actually has, so a companion from a genuinely different model still raises. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent ddaa9b3 commit a5dec82

3 files changed

Lines changed: 142 additions & 8 deletions

File tree

src/modelskill/model/adapters/_res1d.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@
55
import pandas as pd
66

77
if TYPE_CHECKING:
8-
from mikeio1d import Res1D
98
from mikeio1d.result_network import ResultNode, ResultGridPoint, ResultReach
109

10+
from modelskill.network import _Companion
11+
1112
from modelskill.network import NetworkNode, ReachBreakPoint, NetworkReach
1213

1314

@@ -155,7 +156,7 @@ def _build_reach_breakpoints(
155156
length: float | None,
156157
quantities: set[str] | None,
157158
populate_gridpoints: bool,
158-
extra: Res1D | None = None,
159+
extra: _Companion | None = None,
159160
) -> list[ReachBreakPoint]:
160161
"""Build a reach's break points from its mikeio1d gridpoints.
161162
@@ -189,7 +190,9 @@ def _build_reach_breakpoints(
189190
extra_gridpoints = extra.reaches[reach.name].gridpoints
190191

191192
breakpoints: list[ReachBreakPoint] = []
192-
for i, (gp, distances) in enumerate(zip(unique_gridpoints, distances_per_gridpoint)):
193+
for i, (gp, distances) in enumerate(
194+
zip(unique_gridpoints, distances_per_gridpoint)
195+
):
193196
data = _simplify_colnames(gp, quantities) if populate_gridpoints else None
194197
if data is not None and i < len(extra_gridpoints):
195198
data = _merge_extra_quantities(

src/modelskill/network/__init__.py

Lines changed: 87 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,81 @@
7474
}
7575

7676

77+
# The encodings a companion file's text is worth re-reading as. mikeio1d hands
78+
# back '.res' names decoded as UTF-8 but '.resx' names decoded with the Windows
79+
# ANSI codepage, so a name holding a non-ASCII character arrives spelled two
80+
# ways from one model. cp1252 is that codepage on a Western-European Windows.
81+
_COMPANION_ENCODINGS = ("cp1252", "latin-1")
82+
83+
84+
def _repair_mis_decoded(name: str) -> list[str]:
85+
"""Re-read a name as UTF-8, undoing a single-byte decoding of those bytes.
86+
87+
Parameters
88+
----------
89+
name : str
90+
A location name as the companion file reported it.
91+
92+
Returns
93+
-------
94+
list of str
95+
The candidate spellings, which is empty when no encoding round-trips.
96+
A caller must check a candidate against the main file before using it:
97+
the encoding that produced the name is a guess.
98+
"""
99+
candidates = []
100+
for encoding in _COMPANION_ENCODINGS:
101+
try:
102+
repaired = name.encode(encoding).decode("utf-8")
103+
except (UnicodeEncodeError, UnicodeDecodeError):
104+
continue
105+
if repaired != name and repaired not in candidates:
106+
candidates.append(repaired)
107+
return candidates
108+
109+
110+
def _rekey_by_main_file(locations: Any, known: Any) -> dict[str, Any]:
111+
"""Key a companion file's locations by their names in the main result file.
112+
113+
A name that already matches, or that no re-reading reconciles, keeps the
114+
spelling it came with — so a companion from a genuinely different model
115+
still holds names the main file does not, and validation still catches it.
116+
117+
Parameters
118+
----------
119+
locations : mapping of str to location
120+
The companion file's nodes or reaches.
121+
known : container of str
122+
The main file's names for the same kind of location.
123+
124+
Returns
125+
-------
126+
dict of str to location
127+
"""
128+
rekeyed = {}
129+
for name in locations:
130+
key = name
131+
if name not in known:
132+
key = next(
133+
(c for c in _repair_mis_decoded(name) if c in known),
134+
name,
135+
)
136+
rekeyed[key] = locations[name]
137+
return rekeyed
138+
139+
140+
class _Companion:
141+
"""A companion result file, keyed by the main file's location names.
142+
143+
Stands in for the ``Res1D`` it wraps everywhere the loader reaches into a
144+
companion, so a node or reach is found under one spelling of its name.
145+
"""
146+
147+
def __init__(self, res: Res1D, extra: Res1D) -> None:
148+
self.nodes = _rekey_by_main_file(extra.nodes, res.nodes)
149+
self.reaches = _rekey_by_main_file(extra.reaches, res.reaches)
150+
151+
77152
def _check_file_path_is_str(res: Res1D) -> None:
78153
"""Reject a Res1D opened with a path object rather than a string.
79154
@@ -727,9 +802,14 @@ def _read_companion_lengths(inp: str | Path) -> dict[str, float]:
727802
return read_pipe_lengths(path)
728803

729804
@staticmethod
730-
def _open_companion_result(res: Res1D, resx: str | Path | Res1D) -> Res1D:
805+
def _open_companion_result(res: Res1D, resx: str | Path | Res1D) -> _Companion:
731806
"""Open and validate a companion ``.resx`` result file.
732807
808+
Returns
809+
-------
810+
_Companion
811+
The companion's locations, keyed by their names in ``res``.
812+
733813
Raises
734814
------
735815
ValueError
@@ -768,23 +848,25 @@ def _open_companion_result(res: Res1D, resx: str | Path | Res1D) -> Res1D:
768848
f"{len(res.time_index)} ending {res.end_time}."
769849
)
770850

771-
unknown_nodes = set(extra.nodes) - set(res.nodes)
851+
companion = _Companion(res, extra)
852+
853+
unknown_nodes = set(companion.nodes) - set(res.nodes)
772854
if unknown_nodes:
773855
raise ValueError(
774856
f"The '.resx' companion holds nodes {sorted(unknown_nodes)} that are "
775857
"absent from the '.res' network, so the two files do not describe "
776858
"the same model."
777859
)
778860

779-
unknown_reaches = set(extra.reaches) - set(res.reaches)
861+
unknown_reaches = set(companion.reaches) - set(res.reaches)
780862
if unknown_reaches:
781863
raise ValueError(
782864
f"The '.resx' companion holds reaches {sorted(unknown_reaches)} that are "
783865
"absent from the '.res' network, so the two files do not describe "
784866
"the same model."
785867
)
786868

787-
return extra
869+
return companion
788870

789871
@staticmethod
790872
def _validate_extension(
@@ -837,7 +919,7 @@ def _load_res1d_network(
837919
nodes: list[str],
838920
reaches: list[str],
839921
*,
840-
extra: Res1D | None = None,
922+
extra: _Companion | None = None,
841923
lengths: dict[str, float] | None = None,
842924
quantities: set[str] | None = None,
843925
) -> list[Res1DReach]:

tests/test_network.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import shutil
55
import sys
66
from pathlib import Path
7+
from types import SimpleNamespace
78
import pytest
89

910
pytest.importorskip("networkx")
@@ -32,8 +33,11 @@
3233
_EPANET_EXTENSIONS,
3334
_MIKE_EXTENSIONS,
3435
_UNSUPPORTED_EXTENSIONS,
36+
_Companion,
3537
_find_epanet_companions,
3638
_network_from_path,
39+
_repair_mis_decoded,
40+
_rekey_by_main_file,
3741
)
3842
from modelskill.obs import NodeObservation, ReachObservation
3943
from modelskill.quantity import Quantity
@@ -1909,6 +1913,51 @@ def test_unsupported_type_is_refused(self):
19091913
Network.from_epanet(_EPANET_RES, resx=42) # type: ignore[arg-type]
19101914

19111915

1916+
class TestCompanionNameEncoding:
1917+
"""mikeio1d reads '.res' names as UTF-8 and '.resx' names as CP1252.
1918+
1919+
A node called 'ØST' in one file is 'ØST' in the other, so the same model
1920+
looks like two - and the four Danish tank names in a real MIKE+ EPANET
1921+
model were the ones that surfaced it.
1922+
"""
1923+
1924+
def test_a_mis_decoded_name_is_recovered(self):
1925+
assert _repair_mis_decoded("ØST") == ["ØST"]
1926+
assert _repair_mis_decoded("Vandværk_Vest") == ["Vandværk_Vest"]
1927+
1928+
def test_an_ascii_name_has_nothing_to_recover(self):
1929+
assert _repair_mis_decoded("Junction_1") == []
1930+
1931+
def test_a_name_that_no_encoding_explains_is_left_alone(self):
1932+
"""'ØST' is already correct: its bytes are not valid UTF-8 on their own."""
1933+
assert _repair_mis_decoded("ØST") == []
1934+
1935+
def test_a_companion_location_is_keyed_by_the_main_files_name(self):
1936+
rekeyed = _rekey_by_main_file({"ØST": "data"}, {"ØST", "Junction_1"})
1937+
1938+
assert rekeyed == {"ØST": "data"}
1939+
1940+
def test_a_matching_name_is_untouched(self):
1941+
rekeyed = _rekey_by_main_file({"Junction_1": "data"}, {"Junction_1"})
1942+
1943+
assert rekeyed == {"Junction_1": "data"}
1944+
1945+
def test_a_name_from_another_model_keeps_its_own_spelling(self):
1946+
"""Otherwise a genuinely different companion would slip past validation."""
1947+
rekeyed = _rekey_by_main_file({"ØST": "data"}, {"Junction_1"})
1948+
1949+
assert rekeyed == {"ØST": "data"}
1950+
1951+
def test_a_companion_rekeys_both_nodes_and_reaches(self):
1952+
res = SimpleNamespace(nodes={"ØST": 1}, reaches={"Vandværk_Vest": 2})
1953+
extra = SimpleNamespace(nodes={"ØST": 3}, reaches={"Vandværk_Vest": 4})
1954+
1955+
companion = _Companion(res, extra)
1956+
1957+
assert companion.nodes == {"ØST": 3}
1958+
assert companion.reaches == {"Vandværk_Vest": 4}
1959+
1960+
19121961
class TestReadInp:
19131962
"""Minimal .inp reader - see modelskill/model/adapters/_inp.py."""
19141963

0 commit comments

Comments
 (0)