Skip to content

Restore commented-out test assertions from the C# suite - #62

Draft
ryan-kipawa wants to merge 5 commits into
masterfrom
test/restore-axis-unit-assertions
Draft

ryan-kipawa wants to merge 5 commits into
masterfrom
test/restore-axis-unit-assertions

Conversation

@ryan-kipawa

@ryan-kipawa ryan-kipawa commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #49

What and why

Four assertions carried over from the C# test suite were still commented out (#49): three AxisUnit == eumUmeter checks on static-item axes, and a # TODO: Implement? where the Odense ReadTester should read into an existing itemData. The C# 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.

  • 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.
  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.

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 what DfsuBuilder does.
  • 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 codeThis 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 codeThis 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:82-87

assert dfs.ReadItemTimeStep(itemData, j, reshape) is itemData
assert np.shares_memory(itemData.Data, buffer)
assert j == itemData.TimeStepIndex
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.
  • ReadItemTimeStep(itemData, j) returns the itemData it is given, keeps its buffer, sets ItemNumber, 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). 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 (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 6e4252b):

$ 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>.

# 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()`
E               Arrays are not equal
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
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 6e4252b:

$ 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 head only by that comment. Verdict: clean, no must-fix findings.

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 restored 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 6e4252b: ruff, pyrefly, test on ubuntu/windows × 3.11/3.14 and CodeQL. AGENTS.md lists no manual or downstream checks.

Not in this PR

🤖 Generated with Claude Code

@ryan-kipawa
ryan-kipawa force-pushed the test/restore-axis-unit-assertions branch from 83d35bf to 4077005 Compare October 12, 2026 13:24
ryan-kipawa and others added 5 commits October 13, 2026 16:06
DfsFactory.CreateAxisDummy builds its axis with a bare 0 as the unit, so
writing an item on it raises AttributeError ('int' object has no attribute
'value'). The test writes an item on every axis the factory can write
(Dummy, EqD0 to EqD3), reads the file back through the native reader and
checks the axis type, unit and data. Dummy fails until the next commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It now returns DfsAxisEqD1.CreateDummyAxis, the undefined-unit axis that
DfsStaticItem.Create already gives a static item created without an axis,
so writing an item on it no longer raises AttributeError.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The C# suite asserts eumUmeter, which holds for engine-written files but not
for files mikecore builds: their static items get the undefined-unit dummy
axis. The Oresund StaticItemTester now takes an original flag and asserts
the exact unit for each, the way test_dfsu_file.py already does, and a new
test_ReadOresundHDTest runs the testers on the engine-written OresundHD.dfsu
so the eumUmeter branch runs. test_dfs_static_item reads that file, so it
asserts eumUmeter.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The loop carried over from C# had the reuse call commented out with
"TODO: Implement?" and allocated a new itemData each time instead.
ReadItemTimeStep(itemData, j) exists, so the loop now calls it, as the
other ReadTester loops in test_dfsu2D.py and test_dfsu_file.py do. The
values after the loop still match the C# test's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ReadTester loops only check ItemNumber and Time, and data after the
last timestep. This test reads every item-timestep backwards into one
buffer per item, on dfs0, dfs2, dfs3 (also reshaped, through Dfs123File)
and dfsu, and checks that the call returns the given itemData, keeps its
buffer, sets TimeStepIndex and matches ReadItemTimeStep(itemNumber, j).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ryan-kipawa
ryan-kipawa force-pushed the test/restore-axis-unit-assertions branch from 4077005 to 6e4252b Compare October 13, 2026 14:42

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Four test assertions carried over from the C# suite are still commented out

1 participant