Deltares / HYDROLIB-core

Core code around the I/O of the DHYDRO-suite
https://deltares.github.io/HYDROLIB-core/
MIT License
24 stars 4 forks source link

Optional location-related fields should not be written when empty #104

Closed arthurvd closed 1 year ago

arthurvd commented 2 years ago

In the .ext file, locations for Lateral and Boundary objects can be specified in multiple ways, so all these fields are optional. When an MDU+ext file is saved to disk, also all optional fields are written, which is not an error, but it is unnecessarily confusing, nonetheless. Example .ext input:

[Lateral]
    id = lat_871
    name = lat_871
    type = discharge
    locationType = 1d
    branchId = riv_RS1_909
    chainage = 99.678
    discharge = boundaries.bc

[Boundary]
    quantity = waterlevelbnd
    nodeId = 141133.805000_395446.430000
    forcingFile = boundaries.bc

Currently saved output:

[Lateral]
id             = lat_871
name           = lat_871
locationType   = 1d
nodeId         =  # <<< May be omitted
branchId       = riv_RS1_909
chainage       = 99.678
numCoordinates =  # <<< May be omitted
xCoordinates   =  # <<< May be omitted
yCoordinates   =  # <<< May be omitted
discharge      = boundaries.bc
type           = discharge

[Boundary]
quantity     = waterlevelbnd
nodeid       = 141133.805000_395446.430000
locationfile =  # <<< May be omitted
forcingfile  = d:\dam_ar\dflowfm_models\HYDROLIB\MGB_FMonly\fm\saved\boundaries.bc
bndwidth1d   =  # <<< May be omitted
bndbldepth   =  # <<< May be omitted

Note that the same issue arises for Structures (locations) and DamBreaks (waterlevel locations)

priscavdsluis commented 2 years ago

@rhutten, do you know whether it is "safe" to skip empty properties for all files? The MDU is the only INI file I can think of that is OK to write empty properties. Please verify for which files this change can or cannot be implemented.

priscavdsluis commented 2 years ago

@arthurvd, I will change it to a bug, because we should not be serializing models to file that will raise error when deserialized again. Hope you agree with this :)

priscavdsluis commented 2 years ago

@arthurvd and @rhutten, I have made the assumption that empty properties should be written for the MDU file and they should be skipped for all other files. Please someone verify this. Putting this issue for review.