[ryan-kipawa]

Closes #49

What and why

Four TODOs were left in the tests ported from the C# suite (#49): three commented-out AxisUnit == eumUmeter checks on static-item axes, and a # TODO: Implement? where the Odense ReadTester should read into an existing itemData. None of them was a live C# assertion: the two in tests/test_dfsu2D.py are commented-out TODOs in the C# suite too, and the one in tests/test_dfs_static_item.py (the connectivity item, with its 3 * 3636 axis) is in a test that has no C# counterpart. So the assertions here are new ones that complete the TODOs, not restored C# parity. An eumUmeter assertion holds only for engine-written files. Files mikecore builds give static items the undefined-unit dummy axis. So the Oresund StaticItemTester now takes an original flag, the way tests/test_dfsu_file.py already does, and asserts the exact unit for each kind of file. A new test_ReadOresundHDTest runs it on the engine-written file, so both branches run. The Odense loop now calls ReadItemTimeStep(itemData, j). A new test checks that overload against the allocating one on every file type it reaches, including the reshaping Dfs123File override: it starts from a flat buffer and asserts the shape after each read.

• Side finding: DfsFactory.CreateAxisDummy passed a bare 0 as the axis unit, so writing an item on it raised AttributeError: 'int' object has no attribute 'value'. It now returns DfsAxisEqD1.CreateDummyAxis, and a test covers every axis the factory can write.

Commits

1. 0c0e544 Test that each factory axis is written and read back with its unit: one parametrized test over Dummy and EqD0 to EqD3. Check this commit out on its own and only [Dummy] fails.
2. 9012a4c Give CreateAxisDummy an eumUnit instead of a bare 0: a one-line fix.
3. 0476ac9 Restore the static-item axis unit assertions from the C# tests: look at the original flag and the new test_ReadOresundHDTest. Its message is inaccurate: the C# suite does not assert eumUmeter, the line is a commented-out TODO there too. These are new assertions.
4. 3b86e3e Read into the given itemData in the Odense ReadTester: the TODO loop now reads the way the other five ReadTester loops do.
5. 6e4252b Test ReadItemTimeStep(itemData, j) against the allocating overload: test_read_itemtimestep_into_given_itemdata, parametrized over dfs0, dfs2, dfs2 reshaped, dfs3 reshaped and dfsu.
6. e9a97e0 Read into a flat itemData so the reshape cases reshape it: review fix. The buffer used to come from the reshaping read, so the Dfs123File reshape never ran on it. It now starts from CreateEmptyItemData and the test asserts the shape, such as (71, 91) for the reshaped dfs2. Also drops the ItemNumber assertion, which only checked the test's input, and passes the opener functions instead of their names.
7. 6a0c56a Make the Oresund StaticItemTester original flag required: review fix. Every caller passes original explicitly, as in test_dfsu_file.py. Also drops the Odense ItemNumber assertion, for the same reason, and rewords the loop comment so it names no other test.

Code Review Guide

Glossary

• Static item: per-file constant data in a dfsu file, such as node coordinates and connectivity. Each one carries a 1D spatial axis that has a unit.
• Dummy axis: the DfsAxisEqD1 with unit eumUUnitUndefined that DfsAxisEqD1.CreateDummyAxis returns. DfsStaticItem.Create gives it to a static item created without an axis, which is how the static items in the built Oresund copies get it.
• Engine-written file: a file in testdata/ written by the MIKE engine, such as OresundHD.dfsu. Its static-item axes use eumUmeter.
• original: the existing test-helper flag (in test_dfsu_file.py) meaning "this is the engine-written file, not a copy mikecore built".
• ReadItemTimeStep(itemData, j): the overload that fills an existing DfsItemData buffer with item-timestep j and returns it. ReadItemTimeStep(itemNumber, j) allocates a new one.

Where the change sits

flowchart LR
  T1["test_ReadOresundHDTest<br/>tests/test_dfsu2D.py:74"] --> S["FileOresundHDDfsu.StaticItemTester(original)<br/>tests/test_dfsu2D.py:592"]
  T0["test_CreateOresundHDTest<br/>test_CreateOresundHDGenericTest"] --> S
  T2["test_ReadOdenseHD2DTest<br/>test_CreateOdenseHD2DTest"] --> R2["FileOdenseHD2DDfsu.ReadTester<br/>tests/test_dfsu2D.py:1063"]
  T3["test_read_itemtimestep_into_given_itemdata<br/>tests/test_dfs_basic.py:66"] --> RI
  R2 --> RI["DfsFile.ReadItemTimeStep(itemData, j)<br/>mikecore/DfsFile.py"]
  RI --> RN["Dfs123File.ReadItemTimeStepNext<br/>mikecore/Dfs123File.py:21"]
  T4["test_factory_axis_is_written_and_read_back<br/>tests/test_dfsbuilder.py:278"] --> F["DfsFactory.CreateAxisDummy<br/>mikecore/DfsFactory.py:60"]
  F --> D["DfsAxisEqD1.CreateDummyAxis<br/>mikecore/DfsFile.py:414"]
  C["DfsStaticItem.Create<br/>mikecore/DfsFile.py:608"] --> D
  classDef changed fill:#2da44e,stroke:#1a7f37,color:#fff
  class T1,S,R2,T3,T4,F changed

Loading

Green nodes are new or changed. CreateAxisDummy now returns the same axis that DfsStaticItem.Create already uses. Line numbers are at 6e4252b.

Conventions followed

Existing code	This PR
Engine-written and built files assert different axis units, chosen by an original flag
tests/test_dfsu_file.py:324-327 if original: Assert.AreEqual(eumUnit.eumUmeter, axis.AxisUnit) else: Assert.AreEqual(eumUnit.eumUUnitUndefined, axis.AxisUnit)	tests/test_dfsu2D.py:652-655 if original: Assert.AreEqual(eumUnit.eumUmeter, axis.AxisUnit) else: Assert.AreEqual(eumUnit.eumUUnitUndefined, axis.AxisUnit)
A read test on the engine file runs every tester, like the Odense one
tests/test_dfsu2D.py:74-83 def test_ReadOdenseHD2DTest(self): ... FileOdenseHD2DDfsu.StaticItemTester(dfsFile) FileOdenseHD2DDfsu.ReadTester(dfsFile)	tests/test_dfsu2D.py:74-83 def test_ReadOresundHDTest(self): ... FileOresundHDDfsu.StaticItemTester(dfsFile, original=True) FileOresundHDDfsu.ReadTester(dfsFile)
The reuse loop reads the way the other ReadTester loops do
tests/test_dfsu2D.py:778-781 for j in range(12): for i in range(6): itemData = itemDatas[i] dfsFile.ReadItemTimeStep(itemData, j)	tests/test_dfsu2D.py:1065-1068 for j in range(13): for i in range(3): itemData = itemDatas[i] dfsFile.ReadItemTimeStep(itemData, j)
Builder tests reuse compressed_builder_for(axis) and write to tmp_path
tests/test_dfsbuilder.py:220-221 def test_validate_ignores_encode_key_of_dfs0(tmp_path): builder = compressed_builder_for(DfsFactory().CreateAxisEqD0())	tests/test_dfsbuilder.py:278-282 def test_factory_axis_is_written_and_read_back( tmp_path, create, axisType, axisUnit, size ): builder = compressed_builder_for(create(DfsFactory()))

Deviations

Nearby code	This PR
The new tests are pytest functions in pytest files, not Assert.* tester classes. Why: test_dfs_basic.py and test_dfsbuilder.py are plain pytest, and parametrizing needs it. Alternative: add the checks to each ReadTester loop, six copies.
tests/test_dfsu2D.py:798-800 dfsFile.ReadItemTimeStep(itemData, j) Assert.AreEqual(i + 1, itemData.ItemNumber) Assert.AreEqual(j * dtSec, itemData.Time)	tests/test_dfs_basic.py:83-88 assert dfs.ReadItemTimeStep(itemData, j, reshape) is itemData assert np.shares_memory(itemData.Data, buffer) assert shape == itemData.Data.shape assert j == itemData.TimeStepIndex assert expected.Time == itemData.Time np.testing.assert_array_equal(expected.Data, itemData.Data)

Evidence

Claims:

• Engine-written dfsu static items have metre axes; static items in files mikecore builds have undefined-unit axes. Proved by tests/test_dfsu2D.py::Dfsu2DTests::test_ReadOresundHDTest (original=True) and test_CreateOresundHDTest / test_CreateOresundHDGenericTest (original=False), with the oracle below: reading every static item of both files independently of the testers. Flipping the flag either way makes them fail (mutation runs below).
• test_static_item reads the engine file, so eumUmeter is right there: tests/test_dfs_static_item.py::Test_dfs_static_item::test_static_item opens testdata/OresundHD.dfsu. That test has no C# counterpart, so the assertion is new, not restored.
• ReadItemTimeStep(itemData, j) returns the itemData it is given, keeps its buffer and its ItemNumber, sets TimeStepIndex and Time, and fills it with the same values the allocating overload returns, for every item-timestep and on every file type that reaches it (dfs0, dfs2, dfs3, dfsu, with and without the Dfs123File reshape, which turns the flat buffer into the 2D or 3D shape). Proved by tests/test_dfs_basic.py::test_read_itemtimestep_into_given_itemdata, which reads backwards so the buffer holds another timestep before each read, and asserts at least one read changed it. Three mutations of DfsFile.ReadItemTimeStepNext each fail all five cases, and removing the reshape from Dfs123File.ReadItemTimeStepNext fails both reshape cases (below).
• Every axis the factory can write is written and read back with its type, unit and values: tests/test_dfsbuilder.py::test_factory_axis_is_written_and_read_back over Dummy and EqD0 to EqD3, with the native reader as the oracle. The Neq and curvilinear writers in DfsFile.dfsSetItemSpatialAxis are commented out, so those axes can't be written.

Before (the test commit 0c0e544, before the fix):

$ uv run pytest -q tests/test_dfsbuilder.py -k factory_axis
E           AttributeError: 'int' object has no attribute 'value'
mikecore/DfsFile.py:1925: AttributeError
FAILED tests/test_dfsbuilder.py::test_factory_axis_is_written_and_read_back[Dummy]
1 failed, 4 passed, 36 deselected in 0.40s

After (head 6a0c56a):

$ uv run pytest -v tests/test_dfsbuilder.py tests/test_dfs_basic.py tests/test_dfsu2D.py tests/test_dfs_static_item.py -k "factory_axis or into_given or OresundHD or OdenseHD or static_item"
tests/test_dfsbuilder.py::test_factory_axis_is_written_and_read_back[Dummy] PASSED
tests/test_dfsbuilder.py::test_factory_axis_is_written_and_read_back[EqD0] PASSED
tests/test_dfsbuilder.py::test_factory_axis_is_written_and_read_back[EqD1] PASSED
tests/test_dfsbuilder.py::test_factory_axis_is_written_and_read_back[EqD2] PASSED
tests/test_dfsbuilder.py::test_factory_axis_is_written_and_read_back[EqD3] PASSED
tests/test_dfs_basic.py::test_read_itemtimestep_into_given_itemdata[dfs0] PASSED
tests/test_dfs_basic.py::test_read_itemtimestep_into_given_itemdata[dfs2] PASSED
tests/test_dfs_basic.py::test_read_itemtimestep_into_given_itemdata[dfs2-reshape] PASSED
tests/test_dfs_basic.py::test_read_itemtimestep_into_given_itemdata[dfs3-reshape] PASSED
tests/test_dfs_basic.py::test_read_itemtimestep_into_given_itemdata[dfsu] PASSED
tests/test_dfsu2D.py::Dfsu2DTests::test_CreateOdenseHD2DTest PASSED
tests/test_dfsu2D.py::Dfsu2DTests::test_CreateOresundHDGenericTest PASSED
tests/test_dfsu2D.py::Dfsu2DTests::test_CreateOresundHDTest PASSED
tests/test_dfsu2D.py::Dfsu2DTests::test_ReadOdenseHD2DTest PASSED
tests/test_dfsu2D.py::Dfsu2DTests::test_ReadOresundHDTest PASSED
tests/test_dfs_static_item.py::Test_dfs_static_item::test_static_item PASSED

Axis units, read independently of the testers (axis_units.py below, after test_CreateOresundHDTest has built its copy):

$ uv run python -I axis_units.py testdata/OresundHD.dfsu testdata/testtmp/test_build_OresundHD.dfsu
testdata/OresundHD.dfsu: 9 static items, axis units ['eumUmeter']
testdata/testtmp/test_build_OresundHD.dfsu: 9 static items, axis units ['eumUUnitUndefined']

Mutation runs: the new assertions fail when the code or flag is wrong

Each edit was applied alone to the committed head, run, and reverted with git checkout HEAD -- <file>. The ReadItemTimeStepNext mutations were rerun on 6a0c56a with the same counts.

# test_ReadOresundHDTest with original=False (engine file asserted as undefined)
E            ACTUAL: <eumUnit.eumUmeter: 1000>
E            DESIRED: <eumUnit.eumUUnitUndefined: 0>

# StaticItemTester defaulting to original=True (built files asserted as metre), -k CreateOresundHD
E            ACTUAL: <eumUnit.eumUUnitUndefined: 0>
E            DESIRED: <eumUnit.eumUmeter: 1000>

# DfsFile.ReadItemTimeStepNext: `values = itemData.Data` -> `values = item.CreateEmptyItemDataData()`
5 failed, 13 deselected

# DfsFile.ReadItemTimeStepNext: drop `res.TimeStepIndex = self.fpTimeStepIndex`
E               assert 8 == 9
5 failed, 13 deselected

# DfsFile.ReadItemTimeStepNext: drop `res.Time = time`
5 failed, 13 deselected

# Dfs123File.ReadItemTimeStepNext: never reshape (rerun on e9a97e0)
E               assert (71, 91) == (6461,)
E               assert (72, 94, 41) == (277488,)
2 failed, 3 passed, 13 deselected

axis_units.py

import sys

from mikecore.DfsFileFactory import DfsFileFactory

for path in sys.argv[1:]:
    dfs = DfsFileFactory.DfsGenericOpen(path)
    units = set()
    n = 1
    while (item := dfs.ReadStaticItem(n)) is not None:
        units.add(item.SpatialAxis.AxisUnit.name)
        n += 1
    dfs.Close()
    print(f"{path}: {n - 1} static items, axis units {sorted(units)}")

Repo checks (AGENTS.md), on head 6a0c56a:

$ uvx pre-commit run --all-files
ruff check...............................................................Passed
ruff format..............................................................Passed
mixed line ending........................................................Passed
$ uv run pyrefly check
 INFO 0 errors (329 warnings not shown)        # type_check.yml ceiling is 330
$ uv run pytest -q
194 passed, 1 skipped, 8 xfailed, 8 subtests passed in 3.66s

Verify it yourself

gh pr checkout 62
uv sync
uvx pre-commit run --all-files && uv run pyrefly check && uv run pytest -q
git checkout 0c0e544 && uv run pytest -q tests/test_dfsbuilder.py -k factory_axis   # [Dummy] fails
git checkout - && uv run pytest -q tests/test_dfsbuilder.py -k factory_axis         # all pass
uv run python -I axis_units.py testdata/OresundHD.dfsu testdata/testtmp/test_build_OresundHD.dfsu   # script above

Review

• Suggestion: other factory methods would hit the same AttributeError if a caller passed a bare int as axisUnit. Declined: those methods pass through the unit the caller gives, as documented by their signatures. Coercing with eumUnit(...) would change the public contract for every axis type, which is beyond fixing this bug.
• Suggestion: point the Odense reuse loop at the new test so nobody strengthens the loop again. Fixed in 6e4252b (a comment only, no behaviour change).
• One round run, on 0cf56fb, which differs from 6e4252b only by that comment. Verdict: clean, no must-fix findings.
• A second review, on 6e4252b, found that the reshape cases never reshaped the buffer, that two ItemNumber assertions could not fail, that original had a default, and some inaccurate claims in this description. Fixed in e9a97e0 and 6a0c56a, and corrected here.

Risk

• Library change: DfsFactory.CreateAxisDummy only. Its safety rests on eumUnit being an IntEnum. eumUnit.eumUUnitUndefined == 0, so any caller that compared the dummy axis's unit with 0 still gets the same result. Before the fix, writing an item on this axis raised AttributeError (Before), so no working code wrote one. Nothing else in the repo calls it.
• Type check: the new assert eumUnit.eumUmeter == axis.AxisUnit at tests/test_dfs_static_item.py:116 adds one pyrefly missing-attribute warning, the same one each neighbouring line in that block has (staticItem may be None). The count is 329 against the 330 ceiling.
• Tests: test_ReadOresundHDTest runs ReadTester on the engine file as well as the built copies. It passes, so the reader's expected values hold for both.
• CI: every check passes on head 6a0c56a: ruff, pyrefly, test on ubuntu/windows × 3.11/3.14 and CodeQL. AGENTS.md lists no manual or downstream checks.

Not in this PR

• About 30 other commented-out C# assertions remain, on UsedElementCount, FileType and file titles (for example tests/test_dfsu2D.py:624). Four test assertions carried over from the C# suite are still commented out #49 lists four, and these check other properties. UsedElementCount and FileType are not implemented in mikecore (DfsFile.py has # self.FileType = None), so restoring them is new API, not a test fix.
• Re-enabling test_UpdateGeometryOresundHDTest, which is blocked on DfsuFile setters (tests/test_dfsu2D.py:119-120).

🤖 Generated with Claude Code