Write item name, quantity and axis changes to an edited dfs file

[ryan-kipawa]

Closes #38

What and why

Opening a dfs file for editing and assigning ItemInfo[0].Name or ItemInfo[0].Quantity changed only the Python object; closing the file wrote nothing back, so the only way to fix a wrong eumItem/eumUnit was to rewrite the file or click through DataUtility. In the C# library these setters write the item info to the file, and Dfs123File writes a changed SpatialAxis to all items on close; none of that had been ported. This PR ports both. The issue reported it on a dfs2 file, but the setters are in DfsFile, so the fix covers every case that reaches them: every dfs file type (dfs0, dfs1, dfs2, dfs3, dfsu), Edit and Append mode through the typed and the generic factory methods, dynamic items and static items (written by WriteStaticItemData, as in C#), and files being created with DfsBuilder (open in Append mode). As in C#, an axis writes only while it is its item's axis: assigning another SpatialAxis object to an item writes nothing. The file axis is written by Dfs123File, the base of dfs1, dfs2 and dfs3. As decided on the issue, it follows C#: assigning a new Quantity writes it, but changing Quantity.Item/Quantity.Unit in place does not.

• Only an editable file (Edit or Append mode) gets the writes; Read mode behaves as before.
• Flush writes a changed file axis to the dynamic items only, and Close also rewrites the static items. C# rewrites static items on Flush too, but that moves the file pointer, so the next appended time step overwrote time step 0 (review round 1).
• Side finding: DfsuFile.Flush() raised TypeError (it passed itself to DfsFile.Flush), and DfsuFileOpenAppend was the only Open method that required parameters. Test in 9518fd2, fix in ce3e353.

Commits

1. bdf4203 Port the dfs2 item info and item axis modify tests: the two commented-out C# ports, now live; the item info one runs the issue's own example (ExamplesDfs2.ModifyDfs2ItemInfo). Fails on master.
2. abc41a4 Write item name, quantity and axis changes to an edited dfs file: Name/Quantity become properties with an _OnUpdate hook, axes get one through __setattr__, and Dfs123File writes its axis on Flush/Close.
3. 2479afb Test appending after an axis flush, item axis edits and failing setters: fails at abc41a4.
4. c601158 Write a changed axis without moving the file pointer on Flush: the static-item rewrite moves to Close; setters roll back on failure; _Update writes once.
5. e9f111a Test item edits after Close and a failing axis write: fails at c601158 with DFS error 1000.
6. 21cded3 Stop writing item changes after Close and roll back failed axis writes: Close clears the callbacks, since the item pointers point into the freed header.
7. e49e30a Test setting an axis attribute the axis type does not have.
8. 67236bb Allow setting an axis attribute the axis type does not have: rollback handles an unset attribute; changelog wording.
9. a0e6049 Assert initial axis values in dfs2 axis-change tests.
10. a45d04c Test item and axis edits on dfs0, dfs1, dfs3 and dfsu files.
11. 0ba2079 Merge origin/master: no conflicts; brings in the new ruff rules and the file name encoding change.
12. 9518fd2 Test item and axis edits on every dfs file type and way of opening it: the new tests/test_dfs_edit.py, parametrized over file type and opener and checked against invariants (see Evidence). It replaces the per-type copies from commits 3, 5, 7 and 10; the two C# ports stay in test_dfs2.py. Fails only for dfsu, on the side finding.
13. ce3e353 Fix DfsuFile.Flush and let DfsuFileOpenAppend default its parameters.
14. 0c55a8e Test static item edits and item edits while a file is built: fails at the 16 static item cases.
15. 80137e7 Write static item name, quantity and axis changes: static items get the same hooks, Close detaches every hooked item, and the file axis write on Close goes through the hooks.
16. ed1d1f2 Test a Quantity edit on an item whose name cp1252 cannot decode: fails at 80137e7 with UnicodeEncodeError; also checks the static items in test_flush_then_append.
17. af51545 Write back a name cp1252 cannot decode unchanged: the item keeps the name and bytes it read, and sends the bytes while Name is unchanged.
18. d278e70 Test that a dfs1/2/3 file's SpatialAxis is a copy of the item axis: fails with the copy.copy reverted.
19. 4daf251 Test that a static item read from an editable file can be freed: fails at d278e70.
20. b26c945 Let the file drop static items it no longer needs to detach: the hooked items are kept in a weakref.WeakSet.
21. 930d056 Test that a copy of an item or axis from an editable file writes nothing: fails at b26c945 (DFS error 1000 after Close).
22. 7d41b5f Detach a copy of an item or axis from the file: __getstate__ leaves the hook out of a copy.
23. 1ac3f51 Test that a replaced item axis writes nothing: fails at 7d41b5f (DFS error 1000 after Close).
24. e404877 Write an item axis only while it is the item's axis: the hook is bound to its axis and checks item.SpatialAxis is axis; Close detaches every hooked axis.
25. bca52d0 Test that a file axis change reaches an item whose axis was replaced: fails at e404877.
26. 06e9297 Write the file axis to every item's axis in the file: Dfs123File writes to each item's axis as read from the file, not through the item's current object.
27. eba1e3d Test that a bad file axis value is rejected on Flush and Close: passes; it pins the try/finally in Close (fails without it).
28. b366af3, bedcd18 Docstring and changelog wording, and a test check through Flush instead of a field.

Code Review Guide

Glossary

• _OnUpdate: a callback attribute an object calls after one of its values changes. DfsTemporalAxis already uses it to write the time axis to the header.
• Item pointer: a native pointer to one item's info. A dynamic item's points into the file header, which Close destroys; a static item's points into its static vector, which WriteStaticItemData writes.
• dfsUpdateItemInfo: the native call that changes an existing item's eumItem, name and eumUnit. Unlike dfsSetItemInfo it does not take a data type.
• Static item: a per-file data block stored after the header with its own item info and axis; changing it means rewriting the block (WriteStaticItemData).
• Encoded file: a dfs2/dfs3 file whose data is stored compressed (only the cells in a mask), like testdata/OresundHD.dfs3.

Where the change sits

flowchart LR
  A["Dfs2FileOpenEdit, DfsuFileOpenEdit, ...<br/>DfsFileFactory.py:34"] --> B["DfsFile.Open<br/>DfsFile.py"]
  BLD["DfsBuilder.GetFile<br/>DfsFile.Building :976"] --> C
  B --> C["__DynamicItemInfoReadAndCreate<br/>DfsFile.py:1570"]
  B --> RS["ReadStaticItem<br/>__StaticItemReadAndCreate :1613"]
  C --> AT["__AttachItemUpdate<br/>DfsFile.py:1581"]
  RS --> AT
  AT --> D["item.Name / item.Quantity setters<br/>DfsFile.py:602, 620"]
  AT --> E["DfsSpatialAxis.__setattr__<br/>DfsFile.py:412"]
  D --> F["__UpdateItemInfo → dfsUpdateItemInfo<br/>DfsFile.py:1600"]
  E --> G["dfsSetItemSpatialAxis"]
  F --> W["WriteStaticItemData (static)<br/>DfsFile.py:1070"]
  G --> W
  A --> H["Dfs123File.Open<br/>Dfs123File.py:13"]
  H --> I["Dfs123File.Flush / Close<br/>Dfs123File.py:61, 76"]
  I --> J["__WriteSpatialAxis<br/>Dfs123File.py:29"]
  J --> E
  J --> W
  I --> CL["DfsFile.Close detaches items<br/>DfsFile.py:994"]
  classDef changed fill:#2da44e,stroke:#1a7f37,color:#fff
  class C,RS,AT,D,E,F,H,I,J,CL changed

Loading

The setters are the entry points. Every item read from an editable file, dynamic or static, gets its hooks in one place (__AttachItemUpdate). Dfs123File routes a file-level axis change through the same item-axis hooks and, on Close, through the existing static-item writer.

Conventions followed

Change notification through an _OnUpdate callback, attached by the file after reading
DfsFile.py:203-215 def _InvokeOnUpdate(self): if self._OnUpdate is not None: self._OnUpdate() ... def __setTimeUnit(self, timeUnit): self._TimeUnit = timeUnit self._InvokeOnUpdate()	DfsFile.py:620-629 def Quantity(self, quantity): oldQuantity = self._Quantity self._Quantity = quantity try: self._InvokeOnUpdate() ...
The file wires the callback to a private writer that calls the native setter
DfsFile.py:804, 838-839 self.TimeAxis._OnUpdate = self.__UpdateTemporalAxis ... def __UpdateTemporalAxis(self): DfsDLLUtil.dfsSetTemporalAxis( self.DfsFile.headPointer, self.TimeAxis)	DfsFile.py:1581-1607 item._OnUpdate = lambda: self.__UpdateItemInfo(item) axis = item.SpatialAxis ... axis._OnUpdate = lambda: self.__UpdateItemAxis(item, axis) ... rok = DfsDLL.Wrapper.dfsUpdateItemInfo( self.headPointer, item.ItemPointer, ...)
Native signatures declared in DfsDLL.Init
DfsDLL.py:208-212 lib.dfsStaticWrite.argtypes = [ ctypes.c_void_p, ctypes.c_void_p, ctypes.c_void_p, ]	DfsDLL.py:213-220 lib.dfsUpdateItemInfo.argtypes = [ ctypes.c_void_p, ctypes.c_void_p, ctypes.c_int32, ctypes.c_char_p, ctypes.c_int32, ] lib.dfsUpdateItemInfo.restype = ctypes.c_int32
Parametrized pytest tests on a copy in tmp_path, as in the newest test module
test_filename_encoding.py:29-34 @pytest.mark.parametrize("name", NAMES) def test_open_non_ascii_path(tmp_path, name): ... shutil.copy("testdata/OresundHD.dfsu", filename)	test_dfs_edit.py @pytest.mark.parametrize("opener", list(OPENERS)) @pytest.mark.parametrize("edit", list(EDITS)) def test_edit_each_item(case, opener, edit):

Deviations

Axis attributes notify through __setattr__, not properties. Why: the 7 updatable attributes are spread over 9 axis classes as plain fields; one hook keeps them plain. Alternative: a property per attribute per class, like DfsTemporalAxis.
DfsFile.py:210-215 def __setTimeUnit(self, timeUnit): self._TimeUnit = timeUnit self._InvokeOnUpdate() ... TimeUnit = property(__getTimeUnit, __setTimeUnit)	DfsFile.py:412-426 def __setattr__(self, name, value): onUpdate = self.__dict__.get("_OnUpdate") if name not in self._UpdatableAttributes or onUpdate is None: super().__setattr__(name, value) return ...
Callbacks are attached only in Edit/Append mode, whereas the temporal axis gets one in every mode and C# writes in every mode. Why: in Read mode a write only changes the in-memory header, and skipping it keeps names untruncated for read-only users such as MIKE IO. Alternative: attach always, as C# does.
DfsFile.py:804 self.TimeAxis._OnUpdate = self.__UpdateTemporalAxis	DfsFile.py:1585-1586 if self.FileMode not in (DfsFileMode.Edit, DfsFileMode.Append): return
Dfs123File.Flush writes the axis to dynamic items only; the C# library's Flush also rewrites the static items. Why: rewriting static items moves the file pointer, and an append after Flush then overwrote time step 0. Alternative: save and restore the pointer state around the rewrite.
DfsFile.py Flush (the base, unchanged) def Flush(self): self.__CheckIfOpen() DfsDLL.Wrapper.dfsFileFlush( self.headPointer, self.filePointer)	Dfs123File.py:61-74 def Flush(self): """... A changed SpatialAxis is written to the dynamic items here, and to the static items only on Close ...""" if self.filePointer.value is not None: self.__WriteSpatialAxis(staticItems=False) super().Flush()

Evidence

Claims:

• The issue's own example (ExamplesDfs2.ModifyDfs2ItemInfo) now writes the name and quantity: tests/test_dfs2.py::Dfs2Tests::test_ModifyLanduseItemInfoTest.
• Assigning Name, Quantity, SpatialAxis.AxisUnit or SpatialAxis.X0 writes that value to that item only, on every dfs type, in Edit, Append and generic Edit mode, for each item of the file: tests/test_dfs_edit.py::test_edit_each_item (57 cases; dfs0 axes have no X0).
• An edit rewrites only the header, not the data: the file keeps its size, the raw bytes of all dynamic values at the end of the file are unchanged (all but the encoded dfs3), and all data reads back unchanged. Checked after every edit in test_edit_each_item, test_edit_each_static_item, test_name_keeps_its_length_in_the_file and test_file_axis_is_written_to_every_item; at scale by the spike below (20 GB file, < 0.1 s and ≤ 78 KB written per edit, against ≥ 20 s to rewrite it).
• A new name takes the old one's place in the file, padded or truncated to its length, and the item shows the name the file holds: test_name_keeps_its_length_in_the_file, which reads the bytes at the old name's offset in the file, for an empty, 1-character, shorter, same-length and longer name on every type.
• A value the file can't hold (a name outside cp1252, a unit that doesn't fit the item, no quantity, an axis unit that isn't a unit) raises, and the item and the file keep the old value: test_rejected_value_keeps_old_value (byte-identical file).
• A Quantity edit on an item whose name has a byte cp1252 leaves undefined writes the name bytes back unchanged: test_quantity_edit_keeps_a_name_cp1252_cannot_decode (bytes at the name's offset).
• Read mode writes nothing, nor does an edit after Close, nor an in-place Quantity.Item/Unit change: test_read_mode_writes_nothing, test_edit_after_close_writes_nothing, test_quantity_changed_in_place_is_not_written (byte-identical file each).
• A changed static item is written by WriteStaticItemData and not without it, on every type with static items, for each static item: test_edit_each_static_item, test_static_item_edit_is_written_only_by_write_static_item_data.
• A file axis change reaches every dynamic item and every static item with that axis type, and keeps all data: test_file_axis_is_written_to_every_item (dfs1, dfs2, dfs3).
• Flush does not move the file pointer: a time step appended after Flush lands at the end, on every type: test_flush_then_append.
• Edits on a file being built with DfsBuilder are written: test_edit_while_building.
• A dfs1/2/3 file's SpatialAxis is a copy of the first item's axis: changing it reaches the items only on Flush/Close, and never in Read mode: test_file_axis_is_a_copy_of_the_item_axis.
• An axis writes only while it is its item's axis: replacing an item's SpatialAxis writes nothing through the old or the new object, before or after Close: test_replaced_axis_writes_nothing. A copy of an item or axis writes nothing: test_copy_of_an_item_is_not_written.
• A file axis change still reaches an item whose axis object was replaced, of the same or another type: test_file_axis_reaches_an_item_whose_axis_was_replaced.
• A bad file axis value is rejected on Flush and Close, and the file is still closed, unchanged: test_bad_file_axis_value_is_rejected_on_flush_and_close.
• The file does not keep static items it read alive: test_static_item_read_from_an_editable_file_is_freed.

Each claim's test was checked to fail when the claim is false. With all work committed, the code was broken one way at a time and tests/test_dfs_edit.py run, then restored with git checkout HEAD -- <file>:

Broken on purpose	Tests that failed
Callbacks also attached in Read mode	test_read_mode_writes_nothing (5 types)
Name not padded to the old length in memory	test_name_keeps_its_length_in_the_file (20)
Name not rolled back when the write fails	test_rejected_value_keeps_old_value (5)
Axis value not rolled back when the write fails	test_rejected_value_keeps_old_value (5)
Flush also rewrites static items	test_flush_then_append (dfs1, dfs2, dfs3)
Static items not written on Close	test_file_axis_is_written_to_every_item (3), test_ModifyAxisTest
Close keeps item callbacks / doesn't detach an item	test_edit_after_close_writes_nothing (5, DFS error 1000)
AxisUnit not an updatable attribute	test_edit_each_item (15), test_rejected_value_keeps_old_value (5)
File axis not copied from the item axis	test_file_axis_is_a_copy_of_the_item_axis (6)
Axis hook writes whatever axis the item holds (no identity check)	test_replaced_axis_writes_nothing (5)
Close doesn't detach a hooked axis	test_edit_after_close_writes_nothing (5)
No try/finally in Dfs123File.Close	test_bad_file_axis_value_is_rejected_on_flush_and_close (3)

Before: the new and ported tests, run with master's mikecore/ (check out origin/master -- mikecore/ first): uv run pytest -q tests/test_dfs_edit.py tests/test_dfs2.py -k "test_dfs_edit or Modify" -rA

      1 FAILED tests/test_dfs2.py::Dfs2Tests::test_ModifyAxisTest
      1 FAILED tests/test_dfs2.py::Dfs2Tests::test_ModifyLanduseItemInfoTest
      3 FAILED tests/test_dfs_edit.py::test_bad_file_axis_value_is_rejected_on_flush_and_close
      1 FAILED tests/test_dfs_edit.py::test_edit_after_close_writes_nothing
     57 FAILED tests/test_dfs_edit.py::test_edit_each_item
     16 FAILED tests/test_dfs_edit.py::test_edit_each_static_item
      1 FAILED tests/test_dfs_edit.py::test_edit_while_building
      6 FAILED tests/test_dfs_edit.py::test_file_axis_is_a_copy_of_the_item_axis
      3 FAILED tests/test_dfs_edit.py::test_file_axis_is_written_to_every_item
      6 FAILED tests/test_dfs_edit.py::test_file_axis_reaches_an_item_whose_axis_was_replaced
      5 FAILED tests/test_dfs_edit.py::test_flush_then_append
     25 FAILED tests/test_dfs_edit.py::test_name_keeps_its_length_in_the_file
      5 FAILED tests/test_dfs_edit.py::test_quantity_edit_keeps_a_name_cp1252_cannot_decode
     20 FAILED tests/test_dfs_edit.py::test_rejected_value_keeps_old_value
150 failed, 63 passed, 4 skipped, 9 deselected

The 63 that pass on master are the "writes nothing" guards (Read mode, after Close, in-place quantity, copies, a replaced axis, an attribute the axis lacks, a static item without WriteStaticItemData, a freed static item) and two unrelated dfs2 data tests. Master never writes item info, so they guard against this PR writing too much, which the table above shows they catch.

After (bedcd18): uv run pytest -q tests/test_dfs_edit.py tests/test_dfs2.py -k "test_dfs_edit or Modify"

213 passed, 4 skipped, 9 deselected

Full checks at bedcd18: uv run pytest 394 passed, 5 skipped, 8 xfailed; uvx pre-commit run --all-files all Passed; uv run pyrefly check 0 errors, 328 warnings (the same as master; the CI cap is 330).

Spike: edits rewrite only the header, not the file

The issue's files are over 100 GB, so an edit must not rewrite the data. At bedcd18, a dfs2 file of 20 GB (2500 time steps, 2 items of 1000×1000 floats, a 4 MB static item) was edited three times per kind of edit, one edit per process, with nothing else running on the disk. The file is larger than the machine's 15 GB of RAM, so the page cache cannot absorb a full rewrite. On this disk dd with conv=fdatasync wrote 10 GiB in 10.4 s (1.0 GB/s), and creating the file through mikecore took 42.1 s including sync (475 MB/s), so rewriting the file would take at least 20 s. The kernel has no per-process I/O accounting, so the bytes written are the /proc/diskstats count for the whole disk, from just before the edit (after a sync) to just after the sync that follows Close.

Edit (then Close), 3 runs each	Open	Edit + Close	sync	Written to disk, all processes
none (control)	0.9–1.3 ms	0.1 ms	8.6–35.0 ms	20,480 B every run
ItemInfo[0].Name	0.7–1.4 ms	0.1–0.3 ms	4.4–25.2 ms	20,480 B every run
ItemInfo[0].Quantity	0.6–1.8 ms	0.1–0.4 ms	4.3–17.2 ms	20,480 B every run
ItemInfo[1].SpatialAxis.X0 (generic file)	0.8–2.0 ms	0.1–0.3 ms	12.3–58.1 ms	20,480 B every run
SpatialAxis.X0 and .Dy (file axis)	1.0–1.4 ms	12.2–25.3 ms	24.8–35.3 ms	4,018,176 B every run

Every edit finishes, sync included, in at most 62 ms: over 300 times faster than the disk could rewrite the file. The name, quantity and item-axis edits write exactly what the control writes. The file-axis edit writes 3,997,696 B more, the 4,000,000 B static item in whole 4 KiB pages: on Close it rewrites the static item, as C# does, and that data is written back unchanged. Its cost grows with the size of the static items, not with the number of time steps. Afterwards the file reads back with the new names, quantities and axes, all 2500 time steps, an unchanged static item and the expected last time step:

read back: [('Level      ', 'eumIWaterDepth', 55.0, 905.0), ('Depth', 'eumIWaterLevel', 55.0, 905.0)] file axis 55.0 905.0 timesteps 2500
static Bathymetry 55.0 905.0 data ok True
last time step ok True

An earlier run at a45d04c gave the same byte counts. On a 2 GB file, cmp -l against a copy taken before each edit found 9, 3, 1 and 15 changed bytes for the name, quantity, item axis and file axis edits, all at offsets 370 to 855.

Spike script

uv run python spike.py create big.dfs2 2500    # 20 GB
DISK=sdd uv run python spike.py edit big.dfs2 name   # none, name, quantity, itemaxis, fileaxis
uv run python spike.py check big.dfs2

"""Spike: does editing item info / axes in a dfs2 file only rewrite the header?

usage: spike.py create <file> <timesteps>
       spike.py edit <file> <name|quantity|fileaxis|itemaxis|none>
       spike.py check <file>
Set DISK to the /proc/diskstats name of the disk holding the file (default sdd).
"""

import os
import sys
import time

import numpy as np

from mikecore.DfsBuilder import DataValueType, DfsBuilder, DfsSimpleType
from mikecore.DfsFactory import DfsFactory
from mikecore.DfsFileFactory import DfsFileFactory
from mikecore.eum import eumItem, eumQuantity, eumUnit

NX, NY = 1000, 1000  # 4 MB per item per time step


def io():
    # /proc/self/io is not available (no task IO accounting in this kernel), so
    # use the device-wide count of bytes written to the disk holding the file
    with open("/proc/diskstats") as f:
        for line in f:
            parts = line.split()
            if parts[2] == os.environ.get("DISK", "sdd"):
                return {"disk_written": int(parts[9]) * 512}
    raise RuntimeError("DISK not in /proc/diskstats")


def create(path, nt):
    factory = DfsFactory()
    b = DfsBuilder.Create("spike", "spike", 1)
    b.SetDataType(0)
    b.SetGeographicalProjection(factory.CreateProjectionUndefined())
    b.SetTemporalAxis(factory.CreateTemporalEqTimeAxis(eumUnit.eumUsec, 0, 60))
    b.SetSpatialAxis(factory.CreateAxisEqD2(eumUnit.eumUmeter, NX, 0, 10, NY, 0, 10))
    for name in ("Water level", "Depth"):
        item = b.CreateDynamicItemBuilder()
        item.Set(
            name,
            eumQuantity(eumItem.eumIWaterLevel, eumUnit.eumUmeter),
            DfsSimpleType.Float,
        )
        item.SetValueType(DataValueType.Instantaneous)
        item.SetAxis(factory.CreateAxisEqD2(eumUnit.eumUmeter, NX, 0, 10, NY, 0, 10))
        b.AddDynamicItem(item.GetDynamicItemInfo())
    b.CreateFile(path)
    b.AddCreateStaticItem(
        "Bathymetry",
        eumQuantity(eumItem.eumIBathymetry, eumUnit.eumUmeter),
        np.arange(NX * NY, dtype=np.float32),
        factory.CreateAxisEqD2(eumUnit.eumUmeter, NX, 0, 10, NY, 0, 10),
    )
    f = b.GetFile()
    rng = np.random.default_rng(0)
    data = rng.random(NX * NY, dtype=np.float32)
    t0 = time.perf_counter()
    for t in range(nt):
        for _ in range(2):
            f.WriteItemTimeStepNext(t * 60.0, data)
    f.Close()
    os.sync()
    dt = time.perf_counter() - t0
    size = os.path.getsize(path)
    print(
        f"created {size / 1e9:.2f} GB in {dt:.1f} s incl. sync ({size / 1e6 / dt:.0f} MB/s)"
    )


def edit(path, what):
    size = os.path.getsize(path)
    os.sync()  # flush earlier writes so they are not counted
    io0 = io()
    t0 = time.perf_counter()
    if what == "itemaxis":
        f = DfsFileFactory.DfsGenericOpenEdit(path)
    else:
        f = DfsFileFactory.Dfs2FileOpenEdit(path)
    t1 = time.perf_counter()
    if what == "name":
        f.ItemInfo[0].Name = "Level"
    elif what == "quantity":
        f.ItemInfo[0].Quantity = eumQuantity(eumItem.eumIWaterDepth, eumUnit.eumUfeet)
    elif what == "fileaxis":
        f.SpatialAxis.X0 = 55
        f.SpatialAxis.Dy = 905
    elif what == "itemaxis":
        f.ItemInfo[1].SpatialAxis.X0 = 77
    f.Close()
    t2 = time.perf_counter()
    os.sync()
    t3 = time.perf_counter()
    io1 = io()
    written = io1["disk_written"] - io0["disk_written"]
    print(
        f"{what:9} file {size / 1e9:.2f} GB | open {1e3 * (t1 - t0):7.1f} ms, edit+close {1e3 * (t2 - t1):7.1f} ms,"
        f" sync {1e3 * (t3 - t2):7.1f} ms | written to disk incl. other processes {written:>13,} B"
    )


def check(path):
    f = DfsFileFactory.Dfs2FileOpen(path)
    a = f.SpatialAxis
    print(
        "read back:",
        [
            (i.Name, i.Quantity.Item.name, i.SpatialAxis.X0, i.SpatialAxis.Dy)
            for i in f.ItemInfo
        ],
        "file axis",
        a.X0,
        a.Dy,
        "timesteps",
        f.FileInfo.TimeAxis.NumberOfTimeSteps,
    )
    s = f.ReadStaticItemNext()
    print(
        "static",
        s.Name,
        s.SpatialAxis.X0,
        s.SpatialAxis.Dy,
        "data ok",
        np.array_equal(s.Data, np.arange(NX * NY, dtype=np.float32)),
    )
    last = f.ReadItemTimeStep(2, f.FileInfo.TimeAxis.NumberOfTimeSteps - 1).Data
    rng = np.random.default_rng(0)
    print(
        "last time step ok", np.array_equal(last, rng.random(NX * NY, dtype=np.float32))
    )
    f.Close()


cmd = sys.argv[1]
if cmd == "create":
    create(sys.argv[2], int(sys.argv[3]))
elif cmd == "edit":
    edit(sys.argv[2], sys.argv[3])
elif cmd == "check":
    check(sys.argv[2])

Verify it yourself

gh pr checkout 79 && uv sync && mkdir -p testdata/testtmp
uv run pytest tests/test_dfs_edit.py tests/test_dfs2.py -k "test_dfs_edit or Modify"   # after
git checkout origin/master -- mikecore/ && uv run pytest -q tests/test_dfs_edit.py tests/test_dfs2.py -k "test_dfs_edit or Modify"   # before: 150 failed
git checkout HEAD -- mikecore/

The issue's own case, on a copy of testdata/Landuse.dfs2:

cp testdata/Landuse.dfs2 /tmp/l.dfs2 && chmod u+w /tmp/l.dfs2
uv run python -c "
from mikecore.DfsFileFactory import DfsFileFactory as F
from mikecore.eum import eumQuantity, eumItem, eumUnit
f = F.Dfs2FileOpenEdit('/tmp/l.dfs2'); f.ItemInfo[0].Quantity = eumQuantity(eumItem.eumIAreaFraction, eumUnit.eumUPerCent); f.Close()
print(F.Dfs2FileOpen('/tmp/l.dfs2').ItemInfo[0])"
# DfsItem("Landuse", (100311-99001), Float)   on master: (100370-99013)

MIKE Zero 2023

A copy of testdata/Landuse.dfs2 was edited at 67236bb (Name = "Land", Quantity = eumQuantity(eumIAreaFraction, eumUPerCent), SpatialAxis.X0 = 1000, SpatialAxis.Y0 = 2000, then Close) and opened in the MIKE Zero 2023 Grid Editor next to the original. mikecore reads it back as 'Land ' (padded to the old 7 characters), 100311-99001, X0 1000, Y0 2000. MIKE Zero shows the same name and unit: the title is Land and the legend Land [%], where the original shows Landuse [Integer]. The data is unchanged. The dynamic item path it checks has not changed since 67236bb.

Review

Rounds 1 to 4 ran on the first version of the PR (up to a45d04c); rounds 5 to 12 on the merge with master and the commits after it. Round 9 found a must-fix after the ready-pr round limit; the user then asked for it to be fixed (rounds 10 to 12). Round 12 reported no must-fix findings.

Round 1:

• Must fix: Flush after an axis change moved the file pointer, so an appended time step overwrote step 0. Fixed in c601158 (test 2479afb, now test_flush_then_append).
• Close leaked the native handles if the axis write raised. Fixed in c601158 (try/finally).
• Name/Quantity kept the new value when the write failed. Fixed in c601158.
• _Update wrote the axis once per attribute. Fixed in c601158.
• Missing tests for a direct item-axis edit and name padding. Fixed in 2479afb.
• Question, does C# pad a shorter name? Answered: yes, the C# setter keeps the old length and fills with spaces; the native library does the same (test_name_keeps_its_length_in_the_file reads the bytes).
• Question, are newly built files affected? Answered: yes, as in C#; now tested in 0c55a8e.

Round 2:

• Must fix: after Close, the setters and Flush called the native library with item pointers into the freed header. Fixed in 21cded3 (test e9f111a).
• Four pyrefly warnings in the axis test. Fixed in e9f111a.
• The changelog claimed static items are written on flush. Fixed in 21cded3, and reworded in 67236bb.
• An axis write had no rollback. Fixed in 21cded3.
• A stored name with a byte cp1252 leaves undefined decodes to U+FFFD, so a later Quantity write raises UnicodeEncodeError. Declined: the strict encode raises a clear error, whereas a lenient encode would silently rewrite the name to ?. Such names are rare.

Round 3 (no must-fix):

• Changelog mentioned mechanism. Fixed in 67236bb.
• Flush test did not show that Flush writes. Fixed in 67236bb by dropping the claim: in Append mode the native dfsFileFlush does not write item axes.
• getattr raised for an attribute the axis type lacks. Fixed in 67236bb (test e49e30a).
• Question, is it acceptable that Flush differs from C#? Answered: listed under Deviations.
• Question, file.SpatialAxis is now a copy. Answered under Risk.

Round 4 (no must-fix):

• __setattr__ is a second notification pattern. Declined: listed under Deviations with the reason.
• An invalid value on file.SpatialAxis surfaces only at Flush/Close. Declined: C# behaves the same (its file axis only sets a flag), and Close still releases the file through try/finally.
• dfsSetItemSpatialAxis silently skips Neq/CurveLinear axes. Declined: an existing limitation, listed under Not in this PR.

Round 5:

• Must fix: static items inherit the setters but their changes were never written, while C# writes them through WriteStaticItemData. Fixed in 80137e7 (test 0c55a8e).
• Must fix: a file being built with DfsBuilder (Append mode) now gets the hooks, untested and not in the changelog. Fixed: tested in 0c55a8e (test_edit_while_building), changelog in 80137e7. It matches C#, which uses the same setters for files it creates.
• Suggestion: roll back _Update when its write fails. Declined: it can only fail inside Close, after which the items are detached from a closed file, so the in-memory value no longer reaches any file.
• Suggestion: say in Flush why static items wait for Close. Fixed in 80137e7 (docstring).
• Question, the cp1252 re-encode of a name with U+FFFD: answered as in round 2.

Round 6 (the first after the static item fix):

• Must fix: a name with a byte cp1252 leaves undefined is read with U+FFFD, so assigning only a new Quantity, the issue's own operation, raised UnicodeEncodeError. Fixed in af51545 (test ed1d1f2). This supersedes the "declined" answer in rounds 2 and 5.
• Suggestion: check the static items in test_flush_then_append, for the Append-mode static rewrite. Fixed in ed1d1f2.
• Suggestion: note in the changelog that a dfs1/2/3 file's SpatialAxis is now a copy. Fixed in af51545.

Round 7:

• Must fix: no test covers the claim that the file axis is a copy. Fixed in d278e70.
• Suggestion: the file kept every static item read in Edit mode alive until Close. Fixed in b26c945 (test 4daf251).
• Suggestion: assigning a new axis object writes nothing. The changelog says it is the axis's attributes that are written (b26c945); see round 9.
• Suggestion: C# takes the file axis from the first static item when there are no dynamic items. Declined: it changes SpatialAxis in Read mode (from None to an axis), beyond the issue; listed under Not in this PR.
• Suggestion: raise in dfsSetItemSpatialAxis for axis types it doesn't write. Declined: no axis of those types is read today, so none can reach the hook.

Round 8:

• Must fix: a copy of an item or axis kept the original's write hook, so changing it wrote the original's values, and after Close reached the destroyed header. Fixed in 7d41b5f (test 930d056).
• Suggestion: a replaced SpatialAxis is never written. Became round 9's must-fix.
• Suggestion: check axis types before changing any item. Fixed differently in 06e9297, which writes to each item's axis as read from the file.

Round 9:

• Must fix: the axis hook wrote whatever axis the item held, so after item.SpatialAxis = other, the old axis wrote the new one's values and reached the destroyed header after Close. Fixed in e404877 (test 1ac3f51), as the user decided: an axis writes only while it is the item's axis, as in C#, and SpatialAxis stays a plain attribute.
• Suggestion: say that dynamic items get the file axis on Flush too. Fixed in e404877.
• Suggestion: move the SpatialAxis copy note under "Changed". Declined: it is part of the fix for Modify dfs2 items not working #38, so it stays in the Modify dfs2 items not working #38 entry.
• Suggestion: break the item ↔ hook reference cycle with a weak reference. Declined: the cyclic collector frees it, which test_static_item_read_from_an_editable_file_is_freed checks.

Round 10:

• Must fix: the file axis was written through each item's current axis object, so it missed an item whose axis was replaced, and raised for an axis of another type. Fixed in 06e9297 (test bca52d0).
• Suggestion: roll back _Update when its write fails. Declined, as in round 5.
• Suggestion: a bad file axis value is rejected only on Flush/Close. Said in the Flush docstring (06e9297), and tested in round 11.
• Suggestion: static-only files have no file axis. Declined, as in round 7.
• Question, how many pyrefly warnings does the branch add? Answered: none; an export of origin/master has 328 too.

Round 11:

• Must fix: the error path of the file axis write has no test. Fixed in eba1e3d; it fails when the try/finally in Close is removed.
• Suggestion: say that a changed file axis replaces edits made to an item's own axis. Fixed in b366af3 (Flush docstring).

Round 12 (no must-fix):

• Suggestion: the file's SpatialAxis is also a plain attribute. Fixed: the changelog says it is the file axis's attributes that are written (bedcd18).
• Suggestion: static-only files have no file axis. Declined, as in round 7.
• Suggestion: drop "As in the C# library" from the deferred-validation sentence. Fixed in bedcd18.
• Suggestion: check that the file is closed through Flush, not an internal field. Fixed in bedcd18.

Risk

• Read mode is unchanged except for one thing: Dfs123File.SpatialAxis is now a copy of ItemInfo[0].SpatialAxis, not the same object, as in C#. Code that changed one and read the other would see a difference. Nothing in this repo does, and MIKE IO (main) never assigns to SpatialAxis attributes or to an item's Name/Quantity (grep of src/). The MIKE IO downstream workflow at b366af3 (run 38417752079) has the same results as master (run 38416560809); bedcd18 changes only a docstring, the changelog and a test: 978 passed on every job, and the same two known failures, test_eum_conversion (all jobs) and test_select_area_rotated_UTM_2 (Linux only). That workflow always concludes "failure" because of those two.
• Files being built with DfsBuilder now write item edits made before their data, and pad a new name to the built one's length, as C# does (test_edit_while_building). Before, such an edit was kept in memory only.
• Edit/Append safety rests on one fact: dfsUpdateItemInfo and dfsSetItemAxis* only change the header (or, for a static item, its static vector), which Close (or WriteStaticItemData) writes. assertOnlyHeaderChanged in tests/test_dfs_edit.py checks after every edit that the file size and the raw dynamic data bytes are unchanged, and the spike shows it at 20 GB.
• Item pointers after Close are no longer used, because Close detaches every item and axis it hooked, also an axis the item no longer holds, and a copy never carries a hook (test_edit_after_close_writes_nothing, test_replaced_axis_writes_nothing, test_copy_of_an_item_is_not_written, and the mutation table).
• An item's SpatialAxis can still be assigned, as before; the new object is not written, as decided on the PR (C# has no setter). The file axis still reaches that item.
• Not proven here: the encoded dfs3's data is checked only through the reader (its bytes are compressed, so the raw-bytes check doesn't apply); a dfsu 3D or spectral file is not in the matrix, though it goes through the same DfsFile code as the dfsu 2D file that is.
• CI on bedcd18: Lint (run 38418355877), Type check (run 38418355941) and Test Windows and Linux (run 38418355960) all passed.

Not in this PR

• C#'s Dfs123File takes the file axis from the first static item when a dfs1/2/3 file has no dynamic items; here SpatialAxis stays None there, as before. Porting it changes SpatialAxis in Read mode, which is beyond the issue.
• dfsSetItemSpatialAxis has no branch for NeqD1/2/3 and CurveLinear axes (they are commented out), so changing those axes is not written. This is an existing limitation, and porting them needs the coordinate-array marshalling, which roughly doubles the diff.

🤖 Generated with Claude Code