Repository navigation
Restore commented-out test assertions from the C# suite - #62
Draft
ryan-kipawa wants to merge 5 commits into
Draft
ryan-kipawa wants to merge 5 commits into
ryan-kipawa wants to merge 5 commits into
Conversation
ryan-kipawa
force-pushed
the
test/restore-axis-unit-assertions
branch
from
October 12, 2026 13:24
83d35bf to
4077005
Compare
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
force-pushed
the
test/restore-axis-unit-assertions
branch
from
October 13, 2026 14:42
4077005 to
6e4252b
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #49
What and why
Four assertions carried over from the C# test suite were still commented out (#49): three
AxisUnit == eumUmeterchecks on static-item axes, and a# TODO: Implement?where the OdenseReadTestershould read into an existingitemData. The C#eumUmeterassertion holds only for engine-written files. Files mikecore builds give static items the undefined-unit dummy axis. So the OresundStaticItemTesternow takes anoriginalflag, the waytests/test_dfsu_file.pyalready does, and asserts the exact unit for each kind of file. A newtest_ReadOresundHDTestruns it on the engine-written file, so both branches run. The Odense loop now callsReadItemTimeStep(itemData, j). A new test checks that overload against the allocating one on every file type it reaches, including the reshapingDfs123Fileoverride.DfsFactory.CreateAxisDummypassed a bare0as the axis unit, so writing an item on it raisedAttributeError: 'int' object has no attribute 'value'. It now returnsDfsAxisEqD1.CreateDummyAxis, and a test covers every axis the factory can write.Commits
0c0e544Test 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.9012a4cGive CreateAxisDummy an eumUnit instead of a bare 0: a one-line fix.0476ac9Restore the static-item axis unit assertions from the C# tests: look at theoriginalflag and the newtest_ReadOresundHDTest.3b86e3eRead into the given itemData in the Odense ReadTester: theTODOloop now reads the way the other fiveReadTesterloops do.6e4252bTest 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
DfsAxisEqD1with uniteumUUnitUndefinedthatDfsAxisEqD1.CreateDummyAxisreturns.DfsStaticItem.Creategives it to a static item created without an axis, which is whatDfsuBuilderdoes.testdata/written by the MIKE engine, such asOresundHD.dfsu. Its static-item axes useeumUmeter.original: the existing test-helper flag (intest_dfsu_file.py) meaning "this is the engine-written file, not a copy mikecore built".ReadItemTimeStep(itemData, j): the overload that fills an existingDfsItemDatabuffer with item-timestepjand returns it.ReadItemTimeStep(itemNumber, j)allocates a new one.Where the change sits
Green nodes are new or changed.
CreateAxisDummynow returns the same axis thatDfsStaticItem.Createalready uses. Line numbers are at6e4252b.Conventions followed
originalflagtests/test_dfsu_file.py:324-327
tests/test_dfsu2D.py:652-655
tests/test_dfsu2D.py:74-83
tests/test_dfsu2D.py:74-83
ReadTesterloops dotests/test_dfsu2D.py:778-781
tests/test_dfsu2D.py:1065-1068
compressed_builder_for(axis)and write totmp_pathtests/test_dfsbuilder.py:220-221
tests/test_dfsbuilder.py:278-282
Deviations
Assert.*tester classes. Why:test_dfs_basic.pyandtest_dfsbuilder.pyare plain pytest, and parametrizing needs it. Alternative: add the checks to eachReadTesterloop, six copies.tests/test_dfsu2D.py:798-800
tests/test_dfs_basic.py:82-87
Evidence
Claims:
tests/test_dfsu2D.py::Dfsu2DTests::test_ReadOresundHDTest(original=True) andtest_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_itemreads the engine file, soeumUmeteris right there:tests/test_dfs_static_item.py::Test_dfs_static_item::test_static_itemopenstestdata/OresundHD.dfsu.ReadItemTimeStep(itemData, j)returns theitemDatait is given, keeps its buffer, setsItemNumber,TimeStepIndexandTime, 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 theDfs123Filereshape). Proved bytests/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 ofDfsFile.ReadItemTimeStepNexteach fail all five cases (below).tests/test_dfsbuilder.py::test_factory_axis_is_written_and_read_backover Dummy and EqD0 to EqD3, with the native reader as the oracle. The Neq and curvilinear writers inDfsFile.dfsSetItemSpatialAxisare commented out, so those axes can't be written.Before (the test commit
0c0e544, before the fix):After (head
6e4252b):Axis units, read independently of the testers (
axis_units.pybelow, aftertest_CreateOresundHDTesthas built its copy):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>.axis_units.pyRepo checks (AGENTS.md), on head
6e4252b:Verify it yourself
Review
AttributeErrorif a caller passed a bare int asaxisUnit. Declined: those methods pass through the unit the caller gives, as documented by their signatures. Coercing witheumUnit(...)would change the public contract for every axis type, which is beyond fixing this bug.6e4252b(a comment only, no behaviour change).0cf56fb, which differs from head only by that comment. Verdict: clean, no must-fix findings.Risk
DfsFactory.CreateAxisDummyonly. Its safety rests oneumUnitbeing anIntEnum.eumUnit.eumUUnitUndefined == 0, so any caller that compared the dummy axis's unit with0still gets the same result. Before the fix, writing an item on this axis raisedAttributeError(Before), so no working code wrote one. Nothing else in the repo calls it.assert eumUnit.eumUmeter == axis.AxisUnitattests/test_dfs_static_item.py:116adds one pyreflymissing-attributewarning, the same one each neighbouring line in that block has (staticItemmay beNone). The count is 329 against the 330 ceiling.test_ReadOresundHDTestrunsReadTesteron the engine file as well as the built copies. It passes, so the reader's expected values hold for both.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
UsedElementCount,FileTypeand 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.UsedElementCountandFileTypeare not implemented in mikecore (DfsFile.pyhas# self.FileType = None), so restoring them is new API, not a test fix.test_UpdateGeometryOresundHDTest, which is blocked onDfsuFilesetters (tests/test_dfsu2D.py:119-120).🤖 Generated with Claude Code