Fix the remaining type-checking bugs from #66 (#72 T1)

.github/workflows/type_check.yml

@@ -37,7 +37,7 @@ jobs:
         # (the step above checks it); if it crashes, the JSON parse fails.
         shell: bash
         env:
-          MAX_WARNINGS: 330
+          MAX_WARNINGS: 325
         run: |
           warnings=$( (uv run pyrefly check --output-format=json --min-severity=warn --summary=none || true) | uv run python -c "import json, sys; print(sum(e['severity'] == 'warn' for e in json.load(sys.stdin)['errors']))")
           echo "pyrefly warnings: $warnings (ceiling $MAX_WARNINGS)"

CHANGELOG.md

@@ -78,5 +78,62 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#47](https://github.com/DHI/mikecore-python/issues/47)).
 - Reading an item's value type raises the DFS error when the native library
   reports one, instead of returning `DataValueType.Instantaneous`.
+- `eumWrapper.eumGetNextEqvUnit` returns the units equivalent to the base unit.
+  It read the native return code the wrong way round, so it reported no unit
+  where there was one, and raised `ValueError` after the last one, where the
+  native description is null.
+- `DfsuBuilder.SetElementIds` stores the ids, so the file has them instead of
+  the default 1, 2, .... `SetElementIds` and `SetNodeIds` accept int64 arrays
+  and lists and store int32 ids, raising `ValueError` for ids that do not fit in
+  int32. Int64 node ids raised "Data type not supported: int64" in
+  `CreateFile`. `SetElementIds(None)` goes back to the default ids, as
+  `SetNodeIds(None)` does. `SetFromMeshFile` stores a mesh's ids as int32 too,
+  so a mesh from `MeshFile.Create` with int64 ids can be written.
+- `MeshBuilder.SetElementIds(None)` goes back to the default ids instead of
+  raising `TypeError` after `SetElements`, and ids that do not fit in int32
+  raise `ValueError`. They were written to a mesh file that `MeshFile.ReadMesh`
+  could not read.
+- `MeshBuilder.CreateMesh()` makes its default node and element ids int32, as
+  `MeshFile.ReadMesh` reads them. They were int64 on Linux, so the id dtype of
+  a built mesh depended on whether ids were set.
+- The id setters of `DfsuBuilder` and `MeshBuilder` raise `ValueError` for ids
+  that are not a 1-D array, or are booleans. A 2-D array whose size matched
+  the number of nodes or elements, and booleans as 0 and 1, were accepted.
+- `MeshBuilder.SetNodes` keeps z as doubles, as mesh files store it, instead of
+  rounding it to float32. Float32 z passed to `SetNodes`, such as
+  `DfsuFile.Z`, is now written with the double text of those float32 values
+  (`-1.7859922647476196` rather than `-1.7859923`). `MeshBuilder.Create(dfsuFile)`
+  is unchanged.
+- `miketools.Dfs0ToAscii` converts item times from the time axis unit to
+  seconds, so a file with a time unit other than seconds gets the right
+  timestamps. A 10 minute step was written as 10 seconds.
+- `DfsFile.ReadStaticItem` raises `ValueError` for an item number below 1. It
+  returned the first static item, numbered 0 or below. A number past the last
+  item still returns `None`, and so does a number too large for int32, which
+  returned another item or raised `ctypes.ArgumentError`.
+- `DfsFile.ReadItemTimeStepNext` and `ReadItemTimeStep` with `reshape=True`
+  reshape an item with a 2-D or 3-D spatial axis to the axis shape, as
+  `Dfs2File` and `Dfs3File` do. A file opened with
+  `DfsFileFactory.DfsGenericOpen` ignored `reshape`. 0-D and 1-D items stay
+  flat.
+- A `DfsDynamicItemBuilder` can be reused after `GetDynamicItemInfo`. It
+  replaced its item with a blank one but kept its "is set" flags, so the next
+  item passed validation with no name, quantity or value type. The builder now
+  keeps its settings for the next item, and changing it no longer changes the
+  item it returned. Associated static items carry over and add up: after an
+  item with static item 1, `SetAssociatedStaticItem(2)` gives the next item
+  `[1, 2]`.
+- Changing a `DfsStaticItemBuilder` after `GetStaticItem`, or the data array
+  passed to `SetData`, no longer changes the static item it returned. Each
+  returned item has its own copy of the data.
+- `DfsuBuilder` creates `DfsuSpectral0D` files. It raised an `Exception` with
+  no message. The caller supplies the frequency-direction mesh, one element per
+  frequency and direction, and `Validate` reports a Spectral0D builder with no
+  frequencies or directions, or an element count that does not match them, so
+  the file reads back as `DfsuSpectral0D`.
+- `DfsuBuilder.SetFrequencies` and `SetDirections` accept lists and store the
+  values as doubles, as dfsu files store frequencies and directions. A list
+  passed `Validate` and then failed in `CreateFile` with `AttributeError`,
+  leaving a half-written file, and float32 arrays were written as floats.
 
 [Unreleased]: https://github.com/DHI/mikecore-python/compare/v0.3.a1...HEAD

mikecore/DfsBuilder.py

@@ -1,3 +1,4 @@
+import copy
 from typing import Generic, TypeVar
 
 from mikecore.DfsDLL import DfsDLL
@@ -693,7 +694,11 @@ def GetDynamicItemInfo(self):
         # Create and store a clone, such that item-builder can be reused when creating more than one item
         # Otherwise updates to the new item-builder would just update the same item.
         res = self.ItemInfo
-        self.ItemInfo = DfsDynamicItemInfo()
+        self.ItemInfo = copy.copy(res)
+        if res.AssociatedStaticItemNumbers is not None:
+            self.ItemInfo.AssociatedStaticItemNumbers = list(
+                res.AssociatedStaticItemNumbers
+            )
         return res
 
 
@@ -747,4 +752,8 @@ def GetStaticItem(self):
             msgs = DfsBuilder.ErrorMessage(errors)
             raise Exception(msgs)
 
-        return self.ItemInfo
+        # Return a clone with its own data, so neither reusing the builder nor
+        # changing the array given to SetData changes the item returned here.
+        res = copy.copy(self.ItemInfo)
+        res.Data = copy.copy(self.ItemInfo.Data)
+        return res

mikecore/DfsFile.py

@@ -945,10 +945,18 @@ def ReadStaticItem(self, staticItemNo):
 
         :param staticItemNo: Number of static item in the file to read. First static item has number 1
         :returns: The static item number <paramref name="staticItemNo"/>, null if there are not that many static items.
+        :raises ValueError: If <paramref name="staticItemNo"/> is below 1.
         """
         self.__CheckIfOpen()
         if self.fpState == DfsFilePointerState.CreatingItems:
             raise Exception("Can not read static items when file is being created.")
+        if staticItemNo < 1:
+            raise ValueError(
+                f"Static item number must be 1 or more, got {staticItemNo}"
+            )
+        # dfsFindItemStatic takes an int32, and a larger number is past the last item
+        if staticItemNo > np.iinfo(np.int32).max:
+            return None
 
         if (
             self.fpState != DfsFilePointerState.StaticItem
@@ -1064,6 +1072,12 @@ def ReadItemTimeStepNext(
             res = itemData
             res.Time = time
             res.TimeStepIndex = self.fpTimeStepIndex
+        # A 0-D axis has shape (0,) while holding one value, so only reshape 2-D and
+        # 3-D. 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
+        if reshape and axis is not None and len(shape := axis.Shape) >= 2:
+            res.Data = res.Data.reshape(shape, order="F")
         self.__FpDynamicIncrement()
         return res
 

mikecore/DfsuBuilder.py

@@ -3,7 +3,7 @@
 import numpy as np
 from mikecore.DfsFile import DfsProjection, DfsTemporalAxis
 from mikecore.MeshFile import MeshFile
-from mikecore.MeshBuilder import MeshNodes
+from mikecore.MeshBuilder import MeshNodes, _Int32Ids
 from mikecore.DfsFactory import DfsFactory
 from mikecore.DfsBuilder import DfsBuilder
 from mikecore.DfsuFile import (
@@ -74,8 +74,8 @@ def __init__(self, dfsuFileType):
         self.__dynamicItemData = []
 
         # spectral
-        self.__frequencies: Sequence[float] | np.ndarray | None = None
-        self.__directions: Sequence[float] | np.ndarray | None = None
+        self.__frequencies: np.ndarray | None = None
+        self.__directions: np.ndarray | None = None
 
         self.__dfsuFileType = dfsuFileType
         if dfsuFileType == DfsuFileType.Dfsu2D:
@@ -126,10 +126,15 @@ def SetNumberOfSigmaLayers(self, numberOfSigmaLayers: int):
             raise Exception("dfsuFileType")
 
     def SetFrequencies(self, frequencies):
-        self.__frequencies = frequencies
+        # As an array, since the static item writer needs one
+        self.__frequencies = (
+            None if frequencies is None else np.asarray(frequencies, dtype=np.float64)
+        )
 
     def SetDirections(self, directions):
-        self.__directions = directions
+        self.__directions = (
+            None if directions is None else np.asarray(directions, dtype=np.float64)
+        )
 
     # / <summary>
     # / Set a non-standard temporal axis for the dfsu file. WARNING: The dfsu file will not be valid in all contexts.
@@ -226,6 +231,7 @@ def SetNodeIds(self, nodeIds):
         if nodeIds is None:
             self.__nodeIds = None
             return
+        nodeIds = _Int32Ids(nodeIds, "Node")
         if self.__nodes is not None and self.__nodes.x.size != nodeIds.size:
             raise Exception(
                 "Number of node id's does not match number of nodes", "nodeIds"
@@ -270,12 +276,17 @@ def SetElements(self, connectivity):
 
     def SetElementIds(self, elementIds):
         """Set the element id's. Optional. If not set, default values are used (1,2,3,...)"""
+        if elementIds is None:
+            self.__elementIds = None
+            return
+        elementIds = _Int32Ids(elementIds, "Element")
         if (self.__connectivity is not None) and (
             len(self.__connectivity) != elementIds.size
         ):
             raise Exception(
                 "Number of element id's does not match number of elements", "elementIds"
             )
+        self.__elementIds = elementIds
 
     def SetFromMeshFile(self, meshFile: MeshFile):
         """Set projection, nodes and elements from mesh file.
@@ -295,11 +306,15 @@ def SetFromMeshFile(self, meshFile: MeshFile):
             raise ValueError("Mesh file has no projection, nodes or quantity")
         self.__dfsProjection = DfsProjection.Create(wktString)
 
-        self.__nodeIds = meshFile.NodeIds
+        nodeIds = meshFile.NodeIds
+        self.__nodeIds = None if nodeIds is None else _Int32Ids(nodeIds, "Node")
         self.__nodes = MeshNodes(x, y, z.astype(dtype=np.float32), code)
         self.__zUnit = quantity.Unit
 
-        self.__elementIds = meshFile.ElementIds
+        elementIds = meshFile.ElementIds
+        self.__elementIds = (
+            None if elementIds is None else _Int32Ids(elementIds, "Element")
+        )
         self.__connectivity = meshFile.ElementTable
 
     def AddDynamicItem(self, itemName: str, quantity):
@@ -400,8 +415,26 @@ def Validate(self, dieOnError: bool = False):
                 errors.append(
                     "The minimum number of layers is smaller than the number of sigma layers. Element table is invalid"
                 )
+        elif self.__dfsuFileType == DfsuFileType.DfsuSpectral0D:
+            # The mesh is the frequency-direction (rose-plot) geometry, one element
+            # per frequency and direction, as DfsuFile reads it back
+            if self.__frequencies is None and self.__directions is None:
+                errors.append(
+                    "Frequencies or directions must be set for a spectral 0D file"
+                )
+            else:
+                numberOfSpectralValues = 1
+                if self.__frequencies is not None:
+                    numberOfSpectralValues *= len(self.__frequencies)
+                if self.__directions is not None:
+                    numberOfSpectralValues *= len(self.__directions)
+                if len(connectivity) != numberOfSpectralValues:
+                    errors.append(
+                        f"Number of elements ({len(connectivity)}) must equal the "
+                        "number of frequencies times the number of directions "
+                        f"({numberOfSpectralValues}) for a spectral 0D file"
+                    )
         elif self.__dfsuFileType in (
-            DfsuFileType.DfsuSpectral0D,
             DfsuFileType.DfsuSpectral1D,
             DfsuFileType.DfsuSpectral2D,
         ):
@@ -612,8 +645,29 @@ def SetupBuilder(self):
                     np.int32,
                 ),
             )
+        elif self.__dfsuFileType == DfsuFileType.DfsuSpectral0D:
+            # 2 dimensions for a frequency-direction mesh, 1 for frequencies or directions
+            dimensions = (
+                2
+                if self.__frequencies is not None and self.__directions is not None
+                else 1
+            )
+            dfsBuilder.AddCreateCustomBlock(
+                "MIKE_FM",
+                np.array(
+                    [
+                        nodes.x.size,
+                        len(connectivity),
+                        dimensions,
+                        0,
+                        numberOfFrequencies,
+                        numberOfDirections,
+                    ],
+                    np.int32,
+                ),
+            )
         else:
-            raise Exception()
+            raise Exception(f"Dfsu file type {self.__dfsuFileType} not supported")
 
         # For the files with a vertical dimension, the first dynamic item is the Z-coordinate
         if (

mikecore/MeshBuilder.py

@@ -18,6 +18,25 @@ class MeshNodes(NamedTuple):
     code: np.ndarray
 
 
+def _Int32Ids(ids, name):
+    """Ids as the int32 array mesh and dfsu files store, rejecting ids that do not fit.
+
+    Shared by MeshBuilder and DfsuBuilder; name starts the error message.
+    """
+    ids = np.asarray(ids)
+    if ids.ndim != 1:
+        raise ValueError(f"{name} ids must be a 1-D array, got {ids.ndim}-D")
+    if ids.dtype == np.bool_:
+        raise ValueError(f"{name} ids must be integers, not booleans")
+    try:
+        int32Ids = ids.astype(np.int32)
+    except OverflowError:
+        int32Ids = None
+    if int32Ids is None or np.any(int32Ids != ids):
+        raise ValueError(f"{name} ids must be integers that fit in int32")
+    return int32Ids
+
+
 class MeshBuilder:
     def __init__(self):
         self.__projectionString: str | None = None
@@ -26,7 +45,7 @@ def __init__(self):
         self.__nodeIds: np.ndarray | None = None
         self.__nodes: MeshNodes | None = None
 
-        self.__elementIds: Sequence[int] | np.ndarray | None = None
+        self.__elementIds: np.ndarray | None = None
         self.__connectivity: Sequence | np.ndarray | None = None
 
     def SetProjection(self, projection):
@@ -54,7 +73,7 @@ def SetNodes(self, x, y, z, code):
         except Exception as err:
             raise TypeError("y must be array of float") from err
         try:
-            z = np.array(z, dtype=np.float32)
+            z = np.array(z, dtype=np.float64)
         except Exception as err:
             raise TypeError("z must be array of float") from err
         try:
@@ -100,6 +119,10 @@ def SetElements(self, connectivity):
 
     def SetElementIds(self, elementIds):
         """Set the element id's. Optional. If not set, default values are used (1,2,3,...)"""
+        if elementIds is None:
+            self.__elementIds = None
+            return
+        elementIds = _Int32Ids(elementIds, "Element")
         if (self.__connectivity is not None) and (
             len(self.__connectivity) != len(elementIds)
         ):
@@ -149,13 +172,13 @@ def CreateMesh(self) -> MeshFile:
         if self.__eumQuantity is None:
             self.__eumQuantity = eumQuantity(eumItem.eumIBathymetry, eumUnit.eumUmeter)
 
-        # Creating default node id's, if empty
+        # Creating default node id's, if empty, as int32 like MeshFile.ReadMesh
         if self.__nodeIds is None:
-            self.__nodeIds = np.arange(len(nodes.x)) + 1
+            self.__nodeIds = np.arange(1, len(nodes.x) + 1, dtype=np.int32)
 
         # Creating default element id's, if empty
         if self.__elementIds is None:
-            self.__elementIds = np.arange(len(connectivity)) + 1
+            self.__elementIds = np.arange(1, len(connectivity) + 1, dtype=np.int32)
 
         # Creating additional element information
         elementType = np.zeros(len(connectivity), dtype=np.int32)

mikecore/eum.py

@@ -1843,7 +1843,7 @@ def eumGetNextEqvUnit(
         rc = eumDLL.Wrapper.eumGetNextEqvUnit(
             baseunitKey, PrevunitKey, ctypes.byref(unitKey), ctypes.byref(lpUnitDesc)
         )
-        if rc != 0:
+        if rc == 0:
             return False, eumUnit.eumUUnitUndefined, ""
         return True, eumUnit(unitKey.value), eumWrapper.DecodeString(lpUnitDesc)
 

miketools/Dfs0ToAscii.py

@@ -7,6 +7,10 @@ def Dfs0ToAscii(dfs0FileName, txtFileName):
     """Writes dfs0 data to text file in similar format as other MIKE Zero tools"""
 
     dfs = DfsFileFactory.DfsGenericOpen(dfs0FileName)
+    timeAxis = dfs.FileInfo.TimeAxis
+    if timeAxis is None:
+        dfs.Close()
+        raise Exception(f"{dfs0FileName} has no time axis")
     txt = open(txtFileName, "w")
 
     txt.write(" ")
@@ -37,12 +41,12 @@ def Dfs0ToAscii(dfs0FileName, txtFileName):
     # Only set for calendar time axes; otherwise times are written as offsets.
     startDateTime = None
     if (
-        dfs.FileInfo.TimeAxis.TimeAxisType == TimeAxisType.CalendarEquidistant
-        or dfs.FileInfo.TimeAxis.TimeAxisType == TimeAxisType.CalendarNonEquidistant
+        timeAxis.TimeAxisType == TimeAxisType.CalendarEquidistant
+        or timeAxis.TimeAxisType == TimeAxisType.CalendarNonEquidistant
     ):
-        startDateTime = dfs.FileInfo.TimeAxis.StartDateTime
+        startDateTime = timeAxis.StartDateTime
 
-    for i in range(dfs.FileInfo.TimeAxis.NumberOfTimeSteps):
+    for i in range(timeAxis.NumberOfTimeSteps):
         for j in range(len(dfs.ItemInfo)):
             itemData = dfs.ReadItemTimeStepNext()
             if itemData is None:
@@ -51,8 +55,9 @@ def Dfs0ToAscii(dfs0FileName, txtFileName):
                 )
             if j == 0:
                 if startDateTime is not None:
-                    # TODO: Time unit is not always seconds
-                    itemTime = startDateTime + timedelta(seconds=itemData.Time)
+                    itemTime = startDateTime + timedelta(
+                        seconds=timeAxis.ToSeconds(itemData.Time)
+                    )
                     # Depending on the format to write to the file:
                     # txt.write(itemTime.strftime("%Y-%m-%d %H:%M:%S"));         # Seconds accuracy
                     txt.write(