[ryan-kipawa]

Closes #72

What and why

Adding type checking in #66 turned up bugs that were parked as 8 strict xfail tests: eumGetNextEqvUnit reads its return code the wrong way round, and DfsuBuilder.SetElementIds never stores the ids. MeshBuilder rounds z to float32. Dfs0ToAscii treats minutes as seconds, and ReadStaticItem(0) returns the first item. A reused DfsDynamicItemBuilder returns an empty item, and DfsuBuilder can't write Spectral0D files. This PR fixes each one with a test commit and then a fix commit, and removes every xfail from tests/. It also fixes the two related bugs the plan added: reshape=True was ignored on a generic DfsFile, and DfsStaticItemBuilder changed items it had already returned. Two more from #72, SetDeleteValue* and the Validate encode-key index, were fixed in #74.

Scope, by mechanism rather than by the reported case:

• eum: eumGetNextEqvUnit was the only wrapper that read the native result inverted. The other 28 calls in eum.py that check a result all treat non-zero as success, and 2 return counts.
• Ids: every entry point that hands ids to a writer now stores int32 and range-checks them: DfsuBuilder.SetNodeIds, SetElementIds and SetFromMeshFile, and MeshBuilder.SetElementIds, which is the side finding below. SetFromMeshFile matters because MeshFile.Create keeps the ids it is given, so a dfsu built from a mesh with int64 ids failed in CreateFile as well. MeshBuilder.CreateMesh() made its default ids int64 on Linux; they are now int32 too. The setters also reject ids that are not a 1-D array, or are booleans. See "Why int32" below.
• Dfs0ToAscii: tested on every time unit DfsTemporalAxis names (milliseconds, seconds, minutes, hours, days), on both equidistant and non-equidistant calendar axes.
• Reshape: covers ReadItemTimeStepNext and ReadItemTimeStep, on 0-D, 1-D, 2-D and 3-D items. An item with no axis can't come from a file (see Deviations), so that guard is untested.
• Spectral0D: covers frequencies and directions, frequencies only, and directions only, as arrays and as lists.
• ReadStaticItem: covers every number below 1, every item, past the last item, and past int32.

Side findings, each with its own tests:

• DfsuBuilder.SetNodeIds with int64 ids, which is what np.array([...]) gives on Linux, failed in CreateFile with "Data type not supported: int64". Both id setters now store int32, and they raise ValueError for ids that don't fit instead of wrapping them.
• DfsuBuilder.SetElementIds(None) raised; it now goes back to the default ids, as SetNodeIds(None) does.
• MeshBuilder.SetElementIds(None) raised TypeError, although its docstring calls the ids optional. Ids outside int32 were written to a mesh file that MeshFile.ReadMesh then couldn't load. It now behaves like the DfsuBuilder setters.

From the review of 1c48e76, each with its own tests:

• MeshBuilder.CreateMesh() made its default ids int64 on Linux, so a built mesh's id dtype depended on whether ids were set. They are int32 now, as MeshFile.ReadMesh reads them.
• DfsuBuilder.SetFrequencies and SetDirections stored lists as given. Validate passed them, then CreateFile failed with AttributeError and left a half-written file. They are stored as float64 arrays now.
• The id setters accepted 2-D arrays whose size matched, and booleans. They raise ValueError now, through one helper both builders share.
• ReadStaticItem(2**31) wrapped in the untyped ctypes call and returned 'Node id', and 2**64 raised ctypes.ArgumentError. Numbers past int32 return None, as numbers past the last item do.
• GetStaticItem returned the item holding the caller's array, so changing the array passed to SetData changed it. The returned item now has its own copy.

Commits

1. a3f247b Expect eumGetNextEqvUnit to report units while rc is non-zero: removes the 2 eum xfails and adds an oracle test against eumUnitsEqv.
2. 4d5153c Read eumGetNextEqvUnit's return code like the other eum wrappers: rc != 0 → rc == 0.
3. 0d49b2d Expect DfsuBuilder to write the element and node ids it is given: removes the xfail; int32/int64 for both setters.
4. b169d7a Store DfsuBuilder element ids, and node and element ids as int32: the missing self.__elementIds = ....
5. 5a63c43 Expect MeshBuilder to keep z as given: removes the xfail and makes the Oresund rebuild compare z exactly.
6. fdbc893 Keep z as float64 in MeshBuilder.SetNodes: a one-word change. DfsuBuilder and Create(dfsuFile) are left alone.
7. 0cbcc7d Expect Dfs0ToAscii to stamp rows in the time axis unit: removes the xfail; 5 units × 2 calendar axis types.
8. 1b82f89 Convert Dfs0ToAscii item times with the time axis's ToSeconds: also binds the time axis once, which removes 3 pyrefly warnings.
9. 3a52a74 Expect ReadStaticItem to reject item numbers below 1: removes the xfail; tests 0 and -1, and every number from 1 to 9.
10. 6340f38 Reject static item numbers below 1 in ReadStaticItem: the commit message says why the return code stays unchecked.
11. 602237b Test reshape on items read through a generic DfsFile: dfs2/dfs3 must match Dfs2File/Dfs3File. Guards check that 0-D/1-D items, items without an axis, and anything read without reshape stay flat.
12. 7b59a9e Reshape 2-D and 3-D items in DfsFile.ReadItemTimeStepNext: the len(shape) >= 2 rule, applied only when reshape is set and the item has an axis.
13. bee7614 Expect reused item builders to keep settings and leave items alone: removes the xfail; one invariant test over every setter of both builders.
14. 37bf124 Keep a private copy of the item in DfsBuilder's item builders: copy.copy plus its own list or data array.
15. 825d654 Expect DfsuBuilder to write and validate Spectral0D files: removes the xfail; adds frequencies-only and directions-only files and Validate errors.
16. ccf25e3 Write DfsuSpectral0D files in DfsuBuilder: adds the Validate and SetupBuilder branches, and gives the fallback raise a message.
17. dd5e32d Expect SetElementIds(None) to restore defaults, and int32 range checks: the int32 bounds, one past each end, and ids beyond 64 bits.
18. 6f82e0a Restore default element ids on None, and reject ids outside int32: DfsuBuilder.__Int32Ids.
19. 72db4eb Expect MeshBuilder.SetElementIds to take None and reject ids outside int32: side finding.
20. f201509 Restore default mesh element ids on None, and reject ids outside int32: MeshBuilder.__Int32Ids.
21. d5afdac Expect DfsuBuilder to write the ids of a mesh built by MeshBuilder: the third entry point that reaches the id writer.
22. 04707fa Store the ids SetFromMeshFile takes from a mesh as int32: None still gives the defaults.
23. 1c48e76 Lower the pyrefly warning ceiling to 325: master's 328 → 325.

Review fixes on 1c48e76:

24. cc9bc10 Share one int32 id helper between MeshBuilder and DfsuBuilder: _Int32Ids(ids, name) in MeshBuilder.py; messages unchanged.
25. 50c9a3b Expect id setters to reject 2-D, scalar and boolean ids: both builders.
26. 43f3287 Reject 2-D, scalar and boolean ids in the id setters: two checks in the shared helper.
27. 4a4a81e Expect MeshBuilder's default ids to be int32, as MeshFile.ReadMesh reads them: also tests SetFromMeshFile with int64 ids from MeshFile.Create, the path the int32 cast still covers.
28. c8133e0 Make MeshBuilder's default ids int32: np.arange(1, n + 1, dtype=np.int32).
29. d628cc8 Expect Spectral0D files to be written from frequency and direction lists: read back with DfsuFile.
30. ae26532 Store DfsuBuilder frequencies and directions as float64 arrays: np.asarray(..., dtype=np.float64) in the setters.
31. be550ec Drop the reshape test that set an item's axis to None by hand: no file can have such an axis (see Deviations).
32. 7485ae7 Expect ReadStaticItem to return None for numbers past int32: 2³¹−1 (guard), 2³¹, 2³²+1, 2⁶⁴.
33. 10f5585 Return None from ReadStaticItem for numbers past int32: returns before the native call.
34. d366890 Expect a static item to keep its data when the caller changes the array.
35. 3273233 Copy the data into the static item GetStaticItem returns: the copy moves from the builder's item to the returned one.

Code Review Guide

Glossary

• strict xfail: @pytest.mark.xfail(strict=True). The test must fail, and it turns red if it passes. Each one here recorded a suspected bug from Add pyrefly type checking #66.
• MIKE_FM block: the dfsu custom block [nodes, elements, dimensions, layers, (sigma layers | frequencies, directions)]. DfsuFile reads the file type from it and the data type (2001 dfsu, 2002 1-D spectral, 2003 2-D spectral).
• Spectral0D: a point spectrum stored as a dfsu over a frequency-direction (rose-plot) mesh with one element per frequency and direction pair. DfsuFile recognises it as data type 2001 with nf·nd (dims 2), or nf or nd (dims 1), equal to the element count.
• order="F": Fortran (column-major) reshape, which Dfs123File already uses so that x varies fastest.
• Oracle: the independent source a test takes its expected values from (another function, reader or tool), not a recomputation of what the code does.

Where the change sits

flowchart LR
  user([user code / mikeio]) --> eum["eumWrapper.eumGetNextEqvUnit<br/>eum.py:1838"]
  user --> dfsub["DfsuBuilder.SetNodeIds / SetElementIds / SetFromMeshFile<br/>DfsuBuilder.py:229, 277, 291"]
  user --> dfsuv["DfsuBuilder.Validate / SetupBuilder (Spectral0D)<br/>DfsuBuilder.py:418, 648"]
  dfsuv --> dfsuread["DfsuFile type detection<br/>DfsuFile.py:136-165"]
  user --> meshb["MeshBuilder.SetNodes<br/>MeshBuilder.py:63"]
  user --> meshids["MeshBuilder.SetElementIds<br/>MeshBuilder.py:120"]
  meshw -->|CreateMesh| dfsub
  meshb --> meshw["MeshFile.Write / ReadMesh<br/>MeshFile.py"]
  meshids --> meshw
  user --> ascii["Dfs0ToAscii<br/>miketools/Dfs0ToAscii.py:6"]
  ascii --> next
  user --> rsi["DfsFile.ReadStaticItem<br/>DfsFile.py:942"]
  user --> next["DfsFile.ReadItemTimeStepNext<br/>DfsFile.py:1016"]
  d123["Dfs123File.ReadItemTimeStepNext<br/>Dfs123File.py:21"] -->|super, no reshape| next
  user --> dib["DfsDynamicItemBuilder.GetDynamicItemInfo<br/>DfsBuilder.py:687"]
  user --> sib["DfsStaticItemBuilder.GetStaticItem<br/>DfsBuilder.py:748"]
  dib --> addd["DfsBuilder.AddDynamicItem"]
  sib --> adds["DfsBuilder.AddStaticItem"]
  classDef changed fill:#2da44e,stroke:#1a7f37,color:#fff
  class eum,dfsub,dfsuv,meshb,meshids,ascii,rsi,next,dib,sib changed

Loading

Green nodes are changed. DfsuFile, MeshFile and Dfs123File are the unchanged code the changes pair with: the readers that must read the files back, and the override that must keep its own reshape.

Conventions followed

Existing code	This PR
Item numbers below 1 raise ValueError
DfsFile.py:1479 def __DynamicItemInfoReadAndCreate(self, itemNumber, noOfItems): if itemNumber < 1 or itemNumber > noOfItems: raise ValueError(f"Item number must be in the range of 1 - {noOfItems}")	DfsFile.py:953 if staticItemNo < 1: raise ValueError( f"Static item number must be 1 or more, got {staticItemNo}" )
What both builders use lives in MeshBuilder.py, which DfsuBuilder imports
MeshBuilder.py:12 class MeshNodes(NamedTuple): """Node coordinates and codes, one entry per node."""	MeshBuilder.py:21 def _Int32Ids(ids, name): ids = np.asarray(ids) if ids.ndim != 1: raise ValueError(f"{name} ids must be a 1-D array, got {ids.ndim}-D") ...
Spectral MIKE_FM blocks: [nodes, elements, dims, 0, nf, nd]
DfsuBuilder.py:633 elif self.__dfsuFileType == DfsuFileType.DfsuSpectral2D: dfsBuilder.AddCreateCustomBlock( "MIKE_FM", np.array([nodes.x.size, len(connectivity), 2, 0, numberOfFrequencies, numberOfDirections], ...	DfsuBuilder.py:648 elif self.__dfsuFileType == DfsuFileType.DfsuSpectral0D: dimensions = (2 if self.__frequencies is not None and self.__directions is not None else 1) dfsBuilder.AddCreateCustomBlock( "MIKE_FM", np.array([..., dimensions, 0, numberOfFrequencies, numberOfDirections], ...
Reshape in Fortran order to the spatial axis shape
Dfs123File.py:24 res = super().ReadItemTimeStepNext(itemData) if res is not None and (self.reshape or reshape): res.Data = res.Data.reshape(self.SpatialAxis.Shape, order="F")	DfsFile.py:1075 # A 0-D axis has shape (0,) while holding one value, ... axis = item.SpatialAxis if reshape and axis is not None and len(shape := axis.Shape) >= 2: res.Data = res.Data.reshape(shape, order="F")

Deviations

Nearby code	This PR
Question: a reused dynamic item builder keeps its associated static items, so the list grows. Why: the plan says a reused builder "keeps its earlier settings", and associations are settings. Alternative: give the copy an empty list, so each item starts with no associations.
DfsBuilder.py:676 (unchanged) def SetAssociatedStaticItem(self, staticItemNumber): ... self.ItemInfo.AssociatedStaticItemNumbers.append(staticItemNumber)	DfsBuilder.py:697 self.ItemInfo = copy.copy(res) if res.AssociatedStaticItemNumbers is not None: self.ItemInfo.AssociatedStaticItemNumbers = list( res.AssociatedStaticItemNumbers )
Only Spectral0D's frequencies and directions are validated; Spectral1D/2D keep their TODO. Why: only Spectral0D is read back as another type when the counts are wrong, and the plan scoped the check to it. Alternative: validate all three, which needs its own decision.
DfsuBuilder.py:437 elif self.__dfsuFileType in ( DfsuFileType.DfsuSpectral1D, DfsuFileType.DfsuSpectral2D, ): # TODO do we need to check frequency or directions? pass	DfsuBuilder.py:418 elif self.__dfsuFileType == DfsuFileType.DfsuSpectral0D: if self.__frequencies is None and self.__directions is None: errors.append("Frequencies or directions must be set ...") else: ... if len(connectivity) != numberOfSpectralValues:
Reversed: MeshBuilder no longer has its own copy of __Int32Ids. Rounds 1 to 5 declined a shared helper, because each builder kept its private helpers as name-mangled static methods. The review of 1c48e76 asked for one, and the 1-D and boolean checks would otherwise have been written twice, so both builders now call _Int32Ids(ids, name) in MeshBuilder.py (cc9bc10). The messages are unchanged.
DfsuBuilder.py:6 from mikecore.MeshBuilder import MeshNodes, _Int32Ids	MeshBuilder.py:120 def SetElementIds(self, elementIds): ... elementIds = _Int32Ids(elementIds, "Element")
No test for reshaping an item with no axis. Why: no file can have one. DfsFactory can't make EqD4, NeqD or TvarD axes, and a dfs2 given an EqD4 axis through the native dfsSetItemAxisEqD4 fails to open with ValueError: 11 is not a valid SpaceAxisType. GetItemSpatialAxis returns an axis or raises for every SpaceAxisType member. The earlier test set SpatialAxis = None by hand, which CODING_STANDARDS.md doesn't accept, so it was removed (be550ec). The axis is not None guard stays because the return type allows None. Alternative: none that uses a real file.
DfsFile.py:1788 (unchanged) def GetItemSpatialAxis(itemPointer): axisType = SpaceAxisType(DfsDLL.Wrapper.dfsGetItemAxisType(itemPointer)) ... # every member returns an axis or raises return None	DfsFile.py:1075 # ... GetItemSpatialAxis can return None by its type, though a file with an # axis it does not read, such as EqD4, already fails to open; stay flat then. axis = item.SpatialAxis
Spectral0D element shapes are not checked. Why: DfsuFile tells Spectral0D apart by the MIKE_FM counts only, and no public source says which element shapes a frequency-only or direction-only point spectrum uses (a 1-D axis might well use 2-node elements). Alternative: require triangles or quadrilaterals, which needs a file written by a MIKE engine to confirm.
Dfs0ToAscii raises when the file has no time axis (not asked for). Why: it reads the axis once, which removes 3 pyrefly warnings; it used to fail with AttributeError on None. Alternative: assert. It uses a plain Exception, like the function's other raise.
Dfs0ToAscii.py:51 itemData = dfs.ReadItemTimeStepNext() if itemData is None: raise Exception( f"Could not read item {j + 1} at timestep index {i} ..."	Dfs0ToAscii.py:9 dfs = DfsFileFactory.DfsGenericOpen(dfs0FileName) timeAxis = dfs.FileInfo.TimeAxis if timeAxis is None: dfs.Close() raise Exception(f"{dfs0FileName} has no time axis")

Why int32

The id setters and MeshBuilder's defaults use int32 because that is the range the readers enforce. The dfsu and mesh formats don't state a range that this PR relies on. A spike with the public DHI.DFS 23.0.3 NuGet package (C#) on Linux showed:

• The C# MeshFile.ReadMesh rejects a mesh with an id above 2³¹−1 ("failed reading nodes"), and so does Python's MeshFile.ReadMesh.
• A dfsu whose id items are uint32 with id 2³¹ is read by the C# DfsuFile as −2147483648, with no error.
• A dfsu with float64 id items is rejected by the C# reader as "not a valid dfsu file".
• int64 and uint64 ids can't be written: GetDfsType in DfsFile.py has no 64-bit type ("Data type not supported: int64").

Evidence

Claims:

1. eumGetNextEqvUnit lists exactly the units eumUnitsEqv calls equivalent to the base, in eumGetNextUnit order, then returns (False, eumUUnitUndefined, ""). Proved by tests/test_eum.py::test_eumGetNextEqvUnit_lists_every_equivalent_unit (oracle: eumGetNextUnit filtered by eumUnitsEqv, for meter, second, m³/s and °C), ::test_eumGetNextEqvUnit_first_unit and ::test_eumGetNextEqvUnit_after_last_unit.
2. It was the only eum wrapper that read the native result inverted. Proved by the grep under "Checks" below.
3. DfsuBuilder writes the node and element ids it is given (lists, int32 and int64 arrays, the int32 minimum and maximum), and None gives the defaults 1, 2, …. Ids one past either int32 end, or beyond 64 bits, raise ValueError, and so do 2-D arrays, scalars and booleans. Oracle: DfsuFile reading the file back. Proved by tests/test_dfsu_builder.py::test_element_ids_are_written, ::test_node_ids_are_written, ::test_int32_bounds_are_written, ::test_element_ids_set_to_none_use_the_defaults, ::test_ids_outside_int32_are_rejected and ::test_ids_must_be_a_1d_integer_array.
4. DfsuBuilder.SetFromMeshFile writes a mesh's ids, including int64 ids from MeshFile.Create, and a mesh with no ids gets the defaults. Proved by tests/test_dfsu_builder.py::test_ids_from_a_built_mesh_are_written and ::test_int64_ids_from_a_created_mesh_are_written.
5. MeshBuilder.SetElementIds does the same as claim 3, with MeshFile.ReadMesh as the oracle. Proved by tests/test_mesh.py::test_MeshBuilder_element_ids_are_written, ::test_MeshBuilder_element_ids_set_to_none_use_the_defaults, ::test_MeshBuilder_element_ids_outside_int32_are_rejected and ::test_MeshBuilder_element_ids_must_be_a_1d_integer_array. Its default ids have the dtype MeshFile.ReadMesh reads (the oracle), int32: ::test_MeshBuilder_default_ids_are_int32_like_a_read_mesh.
6. MeshBuilder.SetNodes writes z exactly as given. Oracle: testdata/Oresund.mesh itself. tests/test_mesh.py::test_MeshBuilder_rebuilds_the_Oresund_mesh rebuilds it and compares all 2057 z values exactly, and ::test_MeshBuilder_keeps_z_precision checks one 17-digit value. MeshBuilder.Create(dfsuFile) is unchanged: git diff origin/master...HEAD -- mikecore/MeshFile.py is empty, and Create builds through MeshFile.Create, not SetNodes.
7. Dfs0ToAscii stamps each row with the start time plus the item time in the axis unit, for milliseconds, seconds, minutes, hours and days, on equidistant and non-equidistant calendar axes. Oracle: datetime.timedelta. Proved by tests/test_miketools.py::test_Dfs0ToAscii_writes_calendar_times_in_the_time_unit (10 cases; the 2 in seconds also pass on master).
8. ReadStaticItem(n) returns static item n for every n from 1 to the last, raises ValueError below 1, and returns None past the end, including numbers past int32. Oracle: ReadStaticItemNext, read in reverse order so each read has to seek. Proved by tests/test_dfs_basic.py::test_read_static_item_reads_every_item_by_number, ::test_read_static_item_below_one, ::test_read_static_item_past_the_last_item and ::test_read_static_item_past_int32.
9. A generic DfsFile with reshape=True gives the same arrays as Dfs2File and Dfs3File (the oracle). Without reshape, or for 0-D and 1-D items, data stays flat. Proved by tests/test_dfs_basic.py::test_generic_read_reshapes_like_dfs123, ::test_generic_read_stays_flat_without_reshape and ::test_generic_read_does_not_reshape_0d_or_1d. An item with no axis is untested: no file can have one (see Deviations).
10. No setter of either item builder changes an item it already returned. This is an invariant: after every setter, each attribute of the first item is the same object with the same contents. Changing the array passed to SetData doesn't change a returned static item either. A reused dynamic builder keeps its settings. Proved by tests/test_dfsbuilder.py::test_no_builder_setter_changes_a_returned_item, ::test_dynamic_item_builder_can_be_reused, the two test_reused_* tests and ::test_static_item_keeps_its_data_when_the_caller_changes_the_array.
11. DfsuBuilder writes Spectral0D files that DfsuFile reads back as DfsuSpectral0D, with frequencies and directions, frequencies only, and directions only, given as arrays or lists. Validate rejects a missing spectral axis or a mismatched element count. Proved by tests/test_dfsu_builder.py::test_spectral_0d_* (including ::test_spectral_0d_file_from_lists) and ::test_validate_spectral_0d_*, plus the MIKE IO spike below.
12. Dfs2File and Dfs3File reads are unchanged. They call super().ReadItemTimeStepNext(itemData) without reshape, which the existing tests/test_dfs2.py and test_dfs3.py cover.

The same script was run against a git archive of origin/master (fe9021f) and against this branch (1c48e76; the output at 3273233 is identical), from the repo root. Each line is one bug.

Before (master):

1 eumGetNextEqvUnit(meter, first): (False, '')
2 SetElementIds([10, 20]) -> ElementIds: [1, 2]
3 SetNodeIds(int64 [101..104]) -> NodeIds: raises Exception: Data type not supported: int64
4 MeshBuilder z -1.7859922534795478 -> written: -1.7859923
5 Dfs0ToAscii 10-minute step, 2nd row: ['2020-01-01', '00:00:10.000']
6 ReadStaticItem(0): ('Node id', 0)
7 generic dfs2 ReadItemTimeStepNext(reshape=True).shape: (6461,)
8 reused DfsDynamicItemBuilder, 2nd item (Name, Quantity, ValueType): ('', None, None)
9 DfsStaticItemBuilder: 1st item's Name after builder.Set('Second'): Second
10 DfsuBuilder Spectral0D 2 freq x 2 dir -> read back as: raises Exception:
11 MeshBuilder SetElementIds(None); SetElementIds([1, 2**40]): ["raises TypeError: object of type 'NoneType' has no len()", 'raises Exception: Can not load mesh file (failed reading elements): ...']
12 DfsuBuilder.SetFromMeshFile(MeshBuilder().CreateMesh()) -> (NodeIds, ElementIds): raises Exception: Data type not supported: int64

After (this branch):

1 eumGetNextEqvUnit(meter, first): (True, 'meter')
2 SetElementIds([10, 20]) -> ElementIds: [10, 20]
3 SetNodeIds(int64 [101..104]) -> NodeIds: [101, 102, 103, 104]
4 MeshBuilder z -1.7859922534795478 -> written: -1.7859922534795478
5 Dfs0ToAscii 10-minute step, 2nd row: ['2020-01-01', '00:10:00.000']
6 ReadStaticItem(0): raises ValueError: Static item number must be 1 or more, got 0
7 generic dfs2 ReadItemTimeStepNext(reshape=True).shape: (71, 91)
8 reused DfsDynamicItemBuilder, 2nd item (Name, Quantity, ValueType): ('Value', 100000-1000, <DataValueType.Instantaneous: 0>)
9 DfsStaticItemBuilder: 1st item's Name after builder.Set('Second'): First
10 DfsuBuilder Spectral0D 2 freq x 2 dir -> read back as: DfsuSpectral0D
11 MeshBuilder SetElementIds(None); SetElementIds([1, 2**40]): [[1, 2], 'raises ValueError: Element ids must be integers that fit in int32']
12 DfsuBuilder.SetFromMeshFile(MeshBuilder().CreateMesh()) -> (NodeIds, ElementIds): ([1, 2, 3, 4], [1, 2])

The review findings on 1c48e76, with a second script (under the scripts below) run against a git archive of 1c48e76 and against 3273233:

Before (1c48e76):

1 MeshBuilder.CreateMesh() default id dtypes: ('int64', 'int64')
2 Spectral0D SetFrequencies([0.1, 0.2, 0.3]) -> (Validate, type, frequencies): raises AttributeError: 'list' object has no attribute 'size'
4 MeshBuilder.SetElementIds([[10], [20]]); ([True, False]): [[[10], [20]], [1, 0]]
5 ReadStaticItem(2**31): Node id
6 GetStaticItem().Data after the caller sets data[0] = 10: [10.0, 2.0]

After (3273233):

1 MeshBuilder.CreateMesh() default id dtypes: ('int32', 'int32')
2 Spectral0D SetFrequencies([0.1, 0.2, 0.3]) -> (Validate, type, frequencies): ([], 'DfsuSpectral0D', [0.1, 0.2, 0.3])
4 MeshBuilder.SetElementIds([[10], [20]]); ([True, False]): ['raises ValueError: Element ids must be a 1-D array, got 2-D', 'raises ValueError: Element ids must be integers, not booleans']
5 ReadStaticItem(2**31): None
6 GetStaticItem().Data after the caller sets data[0] = 10: [1.0, 2.0]

Finding 3 (the hand-set axis) and finding 8 (the shared helper) change tests and structure only. Finding 7 (Spectral0D element shapes) is left open; see Deviations.

Checks on 3273233 (CI: ruff, pyrefly and tests on Ubuntu and Windows, Python 3.11 and 3.14, all pass):

$ uv run pytest -q
256 passed, 1 skipped, 8 subtests passed      # the skip is the Windows-only code page test
$ grep -rn xfail tests/        # no output
$ uv run pyrefly check
 INFO 0 errors (325 warnings not shown)      # master: 328 (its CI run 38390143370), MAX_WARNINGS 325
$ uvx pre-commit run --all-files             # ruff check, ruff format, mixed line ending: Passed
$ grep -c "0 != eumDLL" mikecore/eum.py; grep -n "rc == 0" mikecore/eum.py
24
1846:        if rc == 0:

Besides those 24 calls and eumGetNextEqvUnit, eumGetItemTypeSeq (0 != iok), eumConvertUnit (iok != 0) and eumGetUnitAbbreviation (1 == ...) also treat non-zero as success. eumGetItemTypeCount and eumGetItemUnitCount return counts.

Each test commit fails without its fix and passes with it. I checked out every test commit, then its fix commit, and ran the tests it adds:

commits  tests                                      test commit          fix commit (full suite)
1/2      test_eum -k EqvUnit                        6 failed             189 passed, 6 xfailed
3/4      test_dfsu_builder -k ids_are_written       3 failed, 1 passed   193 passed, 5 xfailed
5/6      test_mesh -k "z_precision or Oresund"      2 failed, 2 passed   194 passed, 4 xfailed
7/8      test_miketools -k time_unit                8 failed, 2 passed   204 passed, 3 xfailed
9/10     test_dfs_basic -k static_item              2 failed, 4 passed   207 passed, 2 xfailed
11/12    test_dfs_basic -k generic                  2 failed, 5 passed   214 passed, 2 xfailed
13/14    test_dfsbuilder -k "reuse or setter"       4 failed, 1 passed   219 passed, 1 xfailed
15/16    test_dfsu_builder -k spectral              7 failed, 4 passed   226 passed
17/18    test_dfsu_builder -k "none or int32"       5 failed, 5 passed   232 passed
19/20    test_mesh -k element_ids                   5 failed, 4 passed   240 passed
21/22    test_dfsu_builder -k built_mesh            1 failed, 1 passed   242 passed
24       (refactor, no behaviour change)                                 242 passed
25/26    test_mesh test_dfsu_builder -k 1d_integer  6 failed             248 passed
27/28    ... -k "default_ids or int64_ids"          1 failed, 1 passed   250 passed
29/30    test_dfsu_builder -k from_lists            2 failed             252 passed
31       (removes the hand-set axis test)                                251 passed
32/33    test_dfs_basic -k past_int32               3 failed, 1 passed   255 passed
34/35    test_dfsbuilder -k caller_changes          1 failed             256 passed

The passing tests in a test commit are guards that also hold on master, such as the seconds case of the time units, ReadStaticItem numbers from 1 up, 2³¹−1 for ReadStaticItem, and int64 ids from MeshFile.Create, which SetFromMeshFile already cast.

Spike: MIKE IO reads the Spectral0D files (Claim 11). The script writes one file per Spectral0D variant with this branch's DfsuBuilder, then reads it with MIKE IO 3.3.0 from PyPI, with the branch first on PYTHONPATH. Every variant reads as a point spectrum with the axes, times and values written. MIKE IO uses mikecore's DfsuFile to detect the file type, so this independently checks the geometry, axes and data, but not the type detection.

--- freq+dir
  geometry: GeometryFMPointSpectrum
  frequencies: [0.1, 0.2, 0.3]
  directions: [0.0, 90.0]
  shape: (2, 2, 3) dims: ('time', 'direction', 'frequency')
  time: ['2020-01-01 00:00:00', '2020-01-01 01:00:00']
  step 2 values (C order): [2.0, 4.0, 6.0, 8.0, 10.0, 12.0]
--- freq only
  geometry: GeometryFMPointSpectrum
  frequencies: [0.1, 0.2, 0.3]
  directions: None
  shape: (2, 3) dims: ('time', 'frequency')
  step 2 values (C order): [2.0, 4.0, 6.0]
--- dir only
  geometry: GeometryFMPointSpectrum
  frequencies: None
  directions: [0.0, 90.0, 180.0, 270.0]
  shape: (2, 4) dims: ('time', 'direction', 'frequency')
  step 2 values (C order): [2.0, 4.0, 6.0, 8.0]

MIKE IO reports directions in degrees; the file stores the radians passed to SetDirections.

Why ReadStaticItem doesn't check the dfsFindItemStatic return code (OresundHD.dfsu has 9 static items):

n   rc    ReadStaticItem(n) on master
-1  0     ('Node id', -1)
0   0     ('Node id', 0)
9   0     ('Connectivity', 9)
10  0     None
11  1002  None

The return code is 0 one past the end and 1002 further out, and both cases already return None, so it doesn't tell a real error apart from past-the-end.

MIKE IO downstream: test_mikeio.yml (MIKE IO main) ran on this branch (run 39389516723, at 3273233) and on master at the same time (run 39389522212, at fe9021f). Each job's log shows the checkout's mikecore installed. The failing tests are identical, as they were at 1c48e76 (runs 38415673442 and 38415683280):

                     branch                      master
ubuntu 3.12 / 3.14   2 failed, 978 passed        2 failed, 978 passed
windows 3.12 / 3.14  1 failed, 978 passed, 1 sk  1 failed, 978 passed, 1 sk
FAILED tests/test_eum.py::test_eum_conversion                    (all jobs, both runs)
FAILED tests/test_dfs2.py::test_select_area_rotated_UTM_2        (Linux jobs, both runs)

evidence.py

"""Run from the repo root (for testdata/); exercises each #72 bug."""
import datetime, tempfile, os
import numpy as np
from mikecore.eum import eumWrapper, eumUnit, eumQuantity, eumItem
from mikecore.DfsuBuilder import DfsuBuilder
from mikecore.DfsuFile import DfsuFile, DfsuFileType
from mikecore.DfsFactory import DfsFactory
from mikecore.DfsFileFactory import DfsFileFactory
from mikecore.DfsBuilder import DfsBuilder, DfsStaticItemBuilder
from mikecore.DfsFile import DfsSimpleType, DataValueType
from mikecore.MeshBuilder import MeshBuilder
from mikecore.MeshFile import MeshFile
from miketools.Dfs0ToAscii import Dfs0ToAscii

tmp = tempfile.mkdtemp()
def run(name, f):
    try: print(f"{name}: {f()}")
    except Exception as e: print(f"{name}: raises {type(e).__name__}: {e}")

def eqv():
    first = eumWrapper.eumGetNextEqvUnit(eumUnit.eumUmeter, eumUnit.eumUUnitUndefined)
    return first[0], first[2]
run("1 eumGetNextEqvUnit(meter, first)", eqv)

X=[0.0,1.0,1.0,0.0]; Y=[0.0,0.0,1.0,1.0]; EL=[[1,2,3],[1,3,4]]
def dfsu(fileType=DfsuFileType.Dfsu2D):
    b = DfsuBuilder.Create(fileType)
    b.SetProjection(DfsFactory().CreateProjection("NON-UTM"))
    b.SetTimeInfo(datetime.datetime(2020,1,1), 60)
    b.AddDynamicItem("Value", eumQuantity(eumItem.eumIItemUndefined, eumUnit.eumUUnitUndefined))
    return b
def elementIds():
    b = dfsu(); b.SetNodes(X,Y,[0.0]*4,np.zeros(4,np.int32)); b.SetElements(EL)
    b.SetElementIds(np.array([10,20])); fn=os.path.join(tmp,"e.dfsu"); b.CreateFile(fn).Close()
    d=DfsuFile.Open(fn); r=d.ElementIds.tolist(); d.Close(); return r
run("2 SetElementIds([10, 20]) -> ElementIds", elementIds)
def nodeIds():
    b = dfsu(); b.SetNodes(X,Y,[0.0]*4,np.zeros(4,np.int32)); b.SetElements(EL)
    b.SetNodeIds(np.array([101,102,103,104])); fn=os.path.join(tmp,"n.dfsu"); b.CreateFile(fn).Close()
    d=DfsuFile.Open(fn); r=d.NodeIds.tolist(); d.Close(); return r
run("3 SetNodeIds(int64 [101..104]) -> NodeIds", nodeIds)
def meshz():
    b=MeshBuilder(); b.SetProjection("NON-UTM"); b.SetNodes(X,Y,[-1.7859922534795478,-2,-3,-4],[0]*4); b.SetElements(EL)
    fn=os.path.join(tmp,"z.mesh"); b.CreateMesh().Write(fn); return repr(float(MeshFile.ReadMesh(fn).Z[0]))
run("4 MeshBuilder z -1.7859922534795478 -> written", meshz)
def ascii():
    f=DfsFactory(); b=DfsBuilder.Create("t","a",1); b.SetDataType(0)
    b.SetGeographicalProjection(f.CreateProjectionUndefined())
    b.SetTemporalAxis(f.CreateTemporalEqCalendarAxis(eumUnit.eumUminute, datetime.datetime(2020,1,1),0,10))
    i=b.CreateDynamicItemBuilder(); i.Set("V",eumQuantity(eumItem.eumIItemUndefined,eumUnit.eumUUnitUndefined),DfsSimpleType.Float)
    i.SetValueType(DataValueType.Instantaneous); i.SetAxis(f.CreateAxisEqD0()); b.AddDynamicItem(i.GetDynamicItemInfo())
    fn=os.path.join(tmp,"m.dfs0"); b.CreateFile(fn); d=b.GetFile()
    for v in (1.0,2.0): d.WriteItemTimeStepNext(0,np.array([v],np.float32))
    d.Close(); out=os.path.join(tmp,"m.txt"); Dfs0ToAscii(fn,out)
    return open(out).read().splitlines()[-1].split()[:2]
run("5 Dfs0ToAscii 10-minute step, 2nd row", ascii)
def static0():
    d=DfsFileFactory.DfsGenericOpen("testdata/OresundHD.dfsu")
    try: it=d.ReadStaticItem(0); return (it.Name, it.ItemNumber)
    finally: d.Close()
run("6 ReadStaticItem(0)", static0)
def reshape():
    d=DfsFileFactory.DfsGenericOpen("testdata/OresundHD.dfs2")
    r=d.ReadItemTimeStepNext(reshape=True).Data.shape; d.Close(); return r
run("7 generic dfs2 ReadItemTimeStepNext(reshape=True).shape", reshape)
def dyn():
    f=DfsFactory(); i=DfsBuilder.Create().CreateDynamicItemBuilder()
    i.Set("Value",eumQuantity(eumItem.eumIWaterLevel,eumUnit.eumUmeter),DfsSimpleType.Float)
    i.SetValueType(DataValueType.Instantaneous); i.SetAxis(f.CreateAxisEqD0()); i.GetDynamicItemInfo()
    i.SetAxis(f.CreateAxisEqD0()); s=i.GetDynamicItemInfo(); return (s.Name, s.Quantity, s.ValueType)
run("8 reused DfsDynamicItemBuilder, 2nd item (Name, Quantity, ValueType)", dyn)
def st():
    f=DfsFactory(); s=DfsStaticItemBuilder()
    s.Set("First",eumQuantity(eumItem.eumIItemUndefined,eumUnit.eumUUnitUndefined),DfsSimpleType.Float)
    s.SetAxis(f.CreateAxisEqD1(eumUnit.eumUmeter,2,0,1)); s.SetData(np.array([1.0,2.0],np.float32))
    first=s.GetStaticItem(); s.Set("Second",eumQuantity(eumItem.eumIItemUndefined,eumUnit.eumUUnitUndefined),DfsSimpleType.Float)
    return first.Name
run("9 DfsStaticItemBuilder: 1st item's Name after builder.Set('Second')", st)
def sp0():
    b=dfsu(DfsuFileType.DfsuSpectral0D)
    x=np.tile([0.0,1.0,2.0],3); y=np.repeat([0.0,1.0,2.0],3)
    b.SetNodes(x,y,np.zeros(9),np.zeros(9,np.int32)); b.SetElements([[1,2,5,4],[2,3,6,5],[4,5,8,7],[5,6,9,8]])
    b.SetFrequencies(np.array([0.1,0.2])); b.SetDirections(np.array([0.0,np.pi]))
    fn=os.path.join(tmp,"s.dfsu"); b.CreateFile(fn).Close(); d=DfsuFile.Open(fn); r=d.DfsuFileType.name; d.Close(); return r
run("10 DfsuBuilder Spectral0D 2 freq x 2 dir -> read back as", sp0)
def meshIds():
    out = []
    for ids in (None, [1, 2**40]):
        b=MeshBuilder(); b.SetProjection("NON-UTM"); b.SetNodes(X,Y,[0.0]*4,[0]*4); b.SetElements(EL)
        try:
            b.SetElementIds([10, 20]); b.SetElementIds(ids)
            fn=os.path.join(tmp,"ids.mesh"); b.CreateMesh().Write(fn); out.append(MeshFile.ReadMesh(fn).ElementIds.tolist())
        except Exception as e: out.append(f"raises {type(e).__name__}: {str(e)[:70]}")
    return out
run("11 MeshBuilder SetElementIds(None); SetElementIds([1, 2**40])", meshIds)
def fromMesh():
    m=MeshBuilder(); m.SetProjection("NON-UTM"); m.SetNodes(X,Y,[0.0]*4,[0]*4); m.SetElements(EL)
    b=dfsu(); b.SetFromMeshFile(m.CreateMesh()); fn=os.path.join(tmp,"fm.dfsu"); b.CreateFile(fn).Close()
    d=DfsuFile.Open(fn); r=(d.NodeIds.tolist(), d.ElementIds.tolist()); d.Close(); return r
run("12 DfsuBuilder.SetFromMeshFile(MeshBuilder().CreateMesh()) -> (NodeIds, ElementIds)", fromMesh)

review_evidence.py

"""Run from the repo root (for testdata/); exercises each review finding on PR #76."""
import datetime
import os
import tempfile

import numpy as np
from mikecore.DfsBuilder import DfsStaticItemBuilder
from mikecore.DfsFactory import DfsFactory
from mikecore.DfsFile import DfsSimpleType
from mikecore.DfsFileFactory import DfsFileFactory
from mikecore.DfsuBuilder import DfsuBuilder
from mikecore.DfsuFile import DfsuFile, DfsuFileType
from mikecore.eum import eumItem, eumQuantity, eumUnit
from mikecore.MeshBuilder import MeshBuilder

tmp = tempfile.mkdtemp()
X = [0.0, 1.0, 1.0, 0.0]
Y = [0.0, 0.0, 1.0, 1.0]
EL = [[1, 2, 3], [1, 3, 4]]


def run(name, f):
    try:
        print(f"{name}: {f()}")
    except Exception as e:
        print(f"{name}: raises {type(e).__name__}: {e}")


def defaultIds():
    b = MeshBuilder()
    b.SetProjection("NON-UTM")
    b.SetNodes(X, Y, [0.0] * 4, [0] * 4)
    b.SetElements(EL)
    m = b.CreateMesh()
    return (str(m.NodeIds.dtype), str(m.ElementIds.dtype))


run("1 MeshBuilder.CreateMesh() default id dtypes", defaultIds)


def spectralList():
    b = DfsuBuilder.Create(DfsuFileType.DfsuSpectral0D)
    b.SetProjection(DfsFactory().CreateProjection("NON-UTM"))
    b.SetTimeInfo(datetime.datetime(2020, 1, 1), 60)
    b.AddDynamicItem("V", eumQuantity(eumItem.eumIItemUndefined, eumUnit.eumUUnitUndefined))
    b.SetNodes([0.0, 1.0, 2.0, 0.0, 1.0], [0.0, 0.0, 0.0, 1.0, 1.0], np.zeros(5), np.zeros(5, np.int32))
    b.SetElements([[1, 2, 5], [1, 5, 4], [2, 3, 5]])
    b.SetFrequencies([0.1, 0.2, 0.3])
    errors = b.Validate()
    fn = os.path.join(tmp, "s.dfsu")
    b.CreateFile(fn).Close()
    d = DfsuFile.Open(fn)
    r = (errors, d.DfsuFileType.name, d.Frequencies.tolist())
    d.Close()
    return r


run("2 Spectral0D SetFrequencies([0.1, 0.2, 0.3]) -> (Validate, type, frequencies)", spectralList)


def idShapes():
    out = []
    for ids in ([[10], [20]], [True, False]):
        b = MeshBuilder()
        b.SetProjection("NON-UTM")
        b.SetNodes(X, Y, [0.0] * 4, [0] * 4)
        b.SetElements(EL)
        try:
            b.SetElementIds(ids)
            out.append(b.CreateMesh().ElementIds.tolist())
        except Exception as e:
            out.append(f"raises {type(e).__name__}: {e}")
    return out


run("4 MeshBuilder.SetElementIds([[10], [20]]); ([True, False])", idShapes)


def bigStatic():
    d = DfsFileFactory.DfsGenericOpen("testdata/OresundHD.dfsu")
    try:
        it = d.ReadStaticItem(2**31)
        return None if it is None else it.Name
    finally:
        d.Close()


run("5 ReadStaticItem(2**31)", bigStatic)


def staticData():
    data = np.array([1.0, 2.0], np.float32)
    s = DfsStaticItemBuilder()
    s.Set("Item", eumQuantity(eumItem.eumIItemUndefined, eumUnit.eumUUnitUndefined), DfsSimpleType.Float)
    s.SetAxis(DfsFactory().CreateAxisEqD1(eumUnit.eumUmeter, 2, 0, 1))
    s.SetData(data)
    item = s.GetStaticItem()
    data[0] = 10.0
    return item.Data.tolist()


run("6 GetStaticItem().Data after the caller sets data[0] = 10", staticData)

spectral_mikeio.py

"""Write Spectral0D files with this branch's DfsuBuilder and read them with MIKE IO."""

import datetime
import os
import tempfile

import mikecore
import mikeio
import numpy as np
from mikecore.DfsFactory import DfsFactory
from mikecore.DfsuBuilder import DfsuBuilder
from mikecore.DfsuFile import DfsuFileType
from mikecore.eum import eumItem, eumQuantity, eumUnit

print("mikecore", os.path.dirname(mikecore.__file__))
print("mikeio", mikeio.__version__)


def write(fn, frequencies, directions):
    nf = len(frequencies) if frequencies is not None else 1
    nd = len(directions) if directions is not None else 1
    # One quadrilateral per frequency-direction pair on an (nf+1) x (nd+1) grid
    x = np.tile(np.arange(nf + 1, dtype=float), nd + 1)
    y = np.repeat(np.arange(nd + 1, dtype=float), nf + 1)
    elements = []
    for j in range(nd):
        for i in range(nf):
            n = j * (nf + 1) + i + 1
            elements.append([n, n + 1, n + nf + 2, n + nf + 1])
    b = DfsuBuilder.Create(DfsuFileType.DfsuSpectral0D)
    b.SetProjection(DfsFactory().CreateProjection("NON-UTM"))
    b.SetTimeInfo(datetime.datetime(2020, 1, 1), 3600)
    b.SetNodes(x, y, np.zeros(x.size), np.zeros(x.size, np.int32))
    b.SetElements(elements)
    if frequencies is not None:
        b.SetFrequencies(np.array(frequencies))
    if directions is not None:
        b.SetDirections(np.array(directions))
    b.AddDynamicItem(
        "Energy density",
        eumQuantity(eumItem.eumIItemUndefined, eumUnit.eumUUnitUndefined),
    )
    dfsu = b.CreateFile(fn)
    values = np.arange(nf * nd, dtype=np.float32) + 1
    for step in range(2):
        dfsu.WriteItemTimeStepNext(0, values * (step + 1))
    dfsu.Close()
    return values


tmp = tempfile.mkdtemp()
for name, frequencies, directions in [
    ("freq+dir", [0.1, 0.2, 0.3], [0.0, np.pi / 2]),
    ("freq only", [0.1, 0.2, 0.3], None),
    ("dir only", None, [0.0, np.pi / 2, np.pi, 3 * np.pi / 2]),
]:
    fn = os.path.join(tmp, "s.dfsu")
    values = write(fn, frequencies, directions)
    ds = mikeio.read(fn)
    da = ds[0]
    print(f"--- {name}")
    print("  geometry:", type(ds.geometry).__name__)
    print("  frequencies:", None if ds.geometry.frequencies is None else ds.geometry.frequencies.tolist())
    print("  directions:", None if ds.geometry.directions is None else ds.geometry.directions.tolist())
    print("  shape:", da.shape, "dims:", da.dims)
    print("  time:", [str(t) for t in da.time])
    flat = np.asarray(da.values[1]).ravel(order="C")
    print("  step 2 values (C order):", flat.tolist())
    print("  equal to 2*written, some order:", sorted(flat.tolist()) == sorted((2 * values).tolist()))

Verify it yourself

gh pr checkout 76 && uv sync
uv run pytest -q && grep -rn xfail tests/ ; uv run pyrefly check
# Each test commit fails without its fix, e.g.:
git checkout 72db4eb && uv run pytest -q tests/test_mesh.py -k element_ids   # 5 failed
git checkout f201509 && uv run pytest -q tests/test_mesh.py -k element_ids   # passes
# Evidence: save the scripts above to /tmp and, from the repo root:
uv run python /tmp/evidence.py
uv run python /tmp/review_evidence.py
PYTHONPATH=$PWD uv run --with mikeio python /tmp/spectral_mikeio.py

Review

The review of 1c48e76 raised 8 findings. 7 are fixed (commits 24 to 35); finding 7, Spectral0D element shapes, is left open because no public source gives the rule (see Deviations). Finding 6 (where the static item data is copied) is fixed by moving the copy, not removing it: the stated invariant needs one copy per call.

Earlier rounds (5 rounds on 1c48e76, 3 on 933c3b2)

This run re-reviewed the whole rebased branch against the new testing rules (scope by mechanism, proved claims), with a fresh code-reviewer each round.

Round 1: CLEAN (no must-fix).

• Suggestion: the CHANGELOG implied MeshBuilder.Create(dfsuFile) changed: fixed in fdbc893 (it now names float32 z passed to SetNodes, and says Create is unchanged).
• Suggestion: read the axis shape only when reshaping: fixed in 7b59a9e.
• Suggestion: share one __Int32Ids between the builders: declined, see Deviations.
• Suggestion: ids beyond 64 bits raised OverflowError, not ValueError: fixed in 6f82e0a and f201509 (tests dd5e32d and 72db4eb).
• Question: is dims 1 right for frequency-only or direction-only Spectral0D? answered: it is the plan's decision, and MIKE IO reads all three variants as point spectra (spike under Evidence). MIKE Zero is unproven and listed under Risk.
• Question: should associated static items carry over and add up? answered: yes, per the plan's "keeps its earlier settings". The CHANGELOG now says so, and it is a Deviation for the reviewer to confirm.
• Question: where is the MIKE IO run? answered: it runs on the final head, under Evidence.

Round 2: one must-fix.

• Must fix: DfsuBuilder.SetFromMeshFile passed a mesh's int64 ids (from MeshBuilder.CreateMesh()) to the writer, which failed with "Data type not supported: int64": fixed in 04707fa (test d5afdac).
• Suggestion: shared helper: declined, as in round 1.
• Suggestion: narrow MeshBuilder.__elementIds to np.ndarray | None: fixed in f201509.
• Suggestion: say in the CHANGELOG that associated static items add up: fixed in 37bf124.
• Question: is the MeshBuilder.SetElementIds change agreed? answered: it is a side finding under "What and why".

Round 3: one must-fix, declined.

• Must fix: MeshBuilder.SetElementIds is outside the plan: declined. ready-pr's rule for bugs found on the way puts a bug of a few lines, with one clear right answer and a failing test, into the PR when it is in code the PR touches. This PR already changes MeshBuilder.SetNodes, and the fix gives the mesh setter the behaviour the plan approved for DfsuBuilder's id setters. It has its own test and fix commits (72db4eb, f201509) and is named as a side finding. Round 5 raised it again only as a question.
• Suggestion: some axis types are read as None, and reshape=True would then raise AttributeError: fixed in 7b59a9e (test 602237b; removing the guard makes the test fail).
• Suggestion: shared helper: declined.
• Suggestion: the CHANGELOG should say SetElementIds(None) raised only after SetElements: fixed in f201509.
• Suggestion: default mesh ids "may be" int64, depending on NumPy's default int: fixed in 04707fa and d5afdac.
• Suggestion: test or use ValueError for Dfs0ToAscii without a time axis: declined. The plain Exception matches the function's other raise (see Deviations).
• Question: is Spectral0D with dims 1 valid? answered as in round 1.
• Question: where is the MIKE IO run? answered as in round 1.
• Question: does the PR say why the ReadStaticItem return code is unchecked? answered: yes, under Evidence.

Round 4: two must-fix.

• Must fix: the PR body and the MIKE IO run described the old branch: fixed by this body and the run on the final head.
• Must fix: the CHANGELOG said the old eumGetNextEqvUnit returned a unit after the last one. On master it raises ValueError: Native library returned a null string: fixed in 4d5153c.
• Suggestion: shared helper: declined.
• Suggestion: the no-axis comments overstated which axis types read as None: fixed in 7b59a9e and 602237b.
• Suggestion: a CHANGELOG line over 80 columns: fixed in f201509.
• Question: Spectral0D with dims 1: answered as in round 1.

Round 5: CLEAN (no must-fix).

• Suggestion: shared helper: declined.
• Suggestion: NaN ids print a NumPy RuntimeWarning before the ValueError: declined. They are still rejected with ValueError.
• Suggestion: two commits had a different Co-Authored-By line: fixed (dd5e32d, 6f82e0a, message only).
• Suggestion: link Bugs found while adding type checking (#66), not yet fixed #72 from the CHANGELOG bullets: declined. Most bullets in the section, including Enable ruff's bugbear rules and fix what they find #74's, don't link an issue.
• Questions: the MeshBuilder scope and Spectral0D dims 1: answered as above.

5 rounds run; last verdict: CLEAN, covering the head 1c48e76. After round 5, only the two commit messages changed; the tree is identical.

The earlier run's review (3 rounds, on 933c3b2)

Round 1: no must-fix. SetElementIds(None) and the int32 wrap were fixed (now 6f82e0a, test dd5e32d). Dfs0ToAscii now checks the time axis before opening the output file (1b82f89). The directions-only Spectral0D test was added (825d654). Answered the question on associated static items as above.
Round 2: no must-fix. The id helper became a private static method, and the static builder test reuses SetData (bee7614).
Round 3: no must-fix. Declined: polishing the exception type for non-integer ids, and ValueError for the missing time axis.

Risk

• DfsFile.ReadItemTimeStepNext reshaping: this sits under every dfs read in mikecore and mikeio. It is safe because it only acts when reshape=True and the item's axis has 2 or more dimensions, and it reads the axis shape only then. Dfs123File calls super().ReadItemTimeStepNext(itemData) without reshape (Dfs123File.py:24), so dfs2/dfs3 behave as before. Proved by test_dfs2.py, test_generic_read_reshapes_like_dfs123 and test_generic_read_stays_flat_without_reshape.
• MeshBuilder.SetNodes z precision: the text of mesh files written from float32 z passed to SetNodes changes (-1.7859923 becomes -1.7859922647476196). The values are the same float32 numbers, and the CHANGELOG says so.
• Id setters and SetFromMeshFile: they now raise ValueError for ids outside int32, for ids that are not 1-D and for booleans, and convert to int32. SetFromMeshFile raises only for a mesh with such ids, and MeshFile.ReadMesh can't load one, so only a mesh built in memory can reach that. Ids inside int32 are written unchanged, which test_int32_bounds_are_written and test_MeshBuilder_element_ids_are_written prove at both ends of the range.
• Item builders: copy.copy shares Quantity and SpatialAxis with the returned item. That is safe because every setter replaces attributes and none changes them in place (DfsBuilder.py:621-642), which test_no_builder_setter_changes_a_returned_item checks over every setter. The two mutable fields, the associated list and the data array, are copied. The static builder copies the data into the item it returns, once per GetStaticItem, since the builder keeps its own array for the next item and that array can be changed in place through ItemInfo.Data. AddStaticItem writes the data to the file at once, so it keeps no copy.
• MIKE IO: its test suite fails the same 2 known tests on this branch and on master; the runs and the table are under Evidence.
• Spectral frequencies and directions: SetFrequencies and SetDirections now store float64, so a float32 array is written as doubles rather than floats. DfsuFile reads both, and the dfsu static items are documented as doubles in DfsuFile.
• MeshBuilder.CreateMesh() default ids: int32 instead of int64 on Linux. Code that compared the dtype would see a change; the values are the same.
• Unproven: no Spectral0D file written by this builder has been opened in MIKE Zero. Frequency-only and direction-only files are written with MIKE_FM dimensions 1 whatever elements the caller gives. DfsuFile and MIKE IO read them as Spectral0D, but nothing here compares them with a frequency-only point spectrum written by a MIKE engine.

Not in this PR

• Generating the Spectral0D frequency-direction mesh, and mikeio writing point spectra: both out of scope in the plan.
• Dfs0ToAscii writing relative (non-calendar) time offsets in seconds: needs a check against what MIKE Zero writes.
• Validating frequencies and directions for Spectral1D/2D: needs its own decision.
• ReadItemTimeStep*().Time in seconds, and DfsuFile.GetDateTimes ignoring StartTimeOffset: out of scope in the plan.
• Files with an EqD4 or TvarD axis fail to open with ValueError: 11 is not a valid SpaceAxisType, because SpaceAxisType has no member for them: found while looking for a file for the no-axis test.
• DfsuBuilder.SetFromMeshFile stores a mesh's Code as given, so int64 codes from MeshFile.Create fail in CreateFile with "Data type not supported: int64". MeshFile.ReadMesh and MeshBuilder give int32.
• Checking Spectral0D element shapes in Validate (see Deviations).

🤖 Generated with Claude Code