Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Don't write keywords with None values #663

Merged
merged 28 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
3f2ff0d
661: Add integration test that should pass but currently fails.
tim-vd-aardweg Jul 4, 2024
2ba7ed5
661: Added some more tests that should pass but currently fail.
tim-vd-aardweg Jul 4, 2024
ea51da6
661: Implemented potential solution.
tim-vd-aardweg Jul 4, 2024
fdad8b2
661: Improve solution. Deal with possible None values when getting th…
tim-vd-aardweg Jul 4, 2024
ae25d69
661: Remove deprecated keyword dtmassbalance.
tim-vd-aardweg Jul 4, 2024
3342ba4
661: Updated default values for several keywords and updated some des…
tim-vd-aardweg Jul 4, 2024
d4963d2
661: Set default value for sedimentmodelnr.
tim-vd-aardweg Jul 4, 2024
925ec0b
661: Updated testcase reference data.
tim-vd-aardweg Jul 4, 2024
50b5f66
661: Updated testcase to take Unions and Lists into account. This cur…
tim-vd-aardweg Jul 4, 2024
791abcb
661: Updated implementation to take Unions and Lists into account.
tim-vd-aardweg Jul 4, 2024
c6bd0a1
661: Add integration test that should pass but currently fails.
tim-vd-aardweg Jul 4, 2024
5f223a7
661: Added some more tests that should pass but currently fail.
tim-vd-aardweg Jul 4, 2024
c14bbc6
661: Implemented potential solution.
tim-vd-aardweg Jul 4, 2024
cb284ad
661: Improve solution. Deal with possible None values when getting th…
tim-vd-aardweg Jul 4, 2024
a8a8d12
661: Remove deprecated keyword dtmassbalance.
tim-vd-aardweg Jul 4, 2024
2cb3c27
661: Updated default values for several keywords and updated some des…
tim-vd-aardweg Jul 4, 2024
1b1a432
661: Set default value for sedimentmodelnr.
tim-vd-aardweg Jul 4, 2024
890c0ad
661: Updated testcase reference data.
tim-vd-aardweg Jul 4, 2024
bccf7bb
661: Updated testcase to take Unions and Lists into account. This cur…
tim-vd-aardweg Jul 4, 2024
0cbe2b0
661: Updated implementation to take Unions and Lists into account.
tim-vd-aardweg Jul 4, 2024
117f70e
Merge branch 'feat/661_Dont-write-empty-keywords' of https://github.c…
tim-vd-aardweg Jul 4, 2024
443f610
661: Remove obsolete keyword DtMassBalance from tests.
tim-vd-aardweg Jul 4, 2024
6062df1
autoformat: isort & black
tim-vd-aardweg Jul 4, 2024
194850e
661: Implement review comments.
tim-vd-aardweg Jul 5, 2024
5e98579
autoformat: isort & black
tim-vd-aardweg Jul 5, 2024
275f332
Merge branch 'main' into feat/661_Dont-write-empty-keywords
tim-vd-aardweg Jul 5, 2024
c607dd4
661: Update testcase reference data.
tim-vd-aardweg Jul 5, 2024
0b55490
autoformat: isort & black
tim-vd-aardweg Jul 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions hydrolib/core/dflowfm/ini/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,18 @@
from enum import Enum
from math import isnan
from re import compile
from typing import Any, Callable, List, Literal, Optional, Set, Type, Union
from typing import (
Any,
Callable,
List,
Literal,
Optional,
Set,
Type,
Union,
get_args,
get_origin,
)

from pydantic.v1 import Extra, Field, root_validator
from pydantic.v1.class_validators import validator
Expand Down Expand Up @@ -206,7 +217,7 @@ def _to_section(
) -> Section:
props = []
for key, value in self:
if key in self._exclude_fields():
if not self._should_be_serialized(key, value):
continue

field_key = key
Expand All @@ -221,6 +232,39 @@ def _to_section(
props.append(prop)
return Section(header=self._header, content=props)

def _should_be_serialized(self, key: str, value: Any) -> bool:
if key in self._exclude_fields():
return False

field = self.__fields__.get(key)
if not field:
return value is not None

field_type = field.type_
if self._is_union(field_type):
return value is not None or self._union_has_filemodel(field_type)

if self._is_list(field_type):
field_type = get_args(field_type)[0]
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved

return self._value_is_not_none_or_type_is_filemodel(field_type, value)

@staticmethod
def _is_union(field_type: type) -> bool:
return get_origin(field_type) is Union

@staticmethod
def _union_has_filemodel(field_type: type) -> bool:
return any(issubclass(arg, FileModel) for arg in get_args(field_type))

@staticmethod
def _is_list(field_type: type) -> bool:
return get_origin(field_type) is List

@staticmethod
def _value_is_not_none_or_type_is_filemodel(field_type: type, value: Any) -> bool:
return value is not None or issubclass(field_type, FileModel)


Datablock = List[List[Union[float, str]]]

Expand Down
52 changes: 29 additions & 23 deletions hydrolib/core/dflowfm/mdu/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ class Comments(INIBasedModel.Comments):
maxvelocity: float = Field(0.0, alias="maxVelocity")
waterlevelwarn: float = Field(0.0, alias="waterLevelWarn")
tspinupturblogprof: float = Field(0.0, alias="tSpinUpTurbLogProf")
fixedweirtopfrictcoef: Optional[float] = Field(None, alias="fixedWeirTopFrictCoef")
fixedweirtopfrictcoef: Optional[float] = Field(-999, alias="fixedWeirTopFrictCoef")
fixedweir1d2d_dx: float = Field(50.0, alias="fixedWeir1D2D_dx")
junction1d: int = Field(0, alias="junction1D")
fixedweirtopwidth: float = Field(3.0, alias="fixedWeirTopWidth")
Expand Down Expand Up @@ -586,7 +586,7 @@ class Comments(INIBasedModel.Comments):
idensform: int = Field(2, alias="idensform")
ag: float = Field(9.81, alias="ag")
tidalforcing: bool = Field(False, alias="tidalForcing")
itcap: Optional[float] = Field(None, alias="ITcap")
itcap: Optional[float] = Field(0.0, alias="ITcap")
doodsonstart: float = Field(55.565, alias="doodsonStart")
doodsonstop: float = Field(375.575, alias="doodsonStop")
doodsoneps: float = Field(0.0, alias="doodsonEps")
Expand Down Expand Up @@ -635,7 +635,7 @@ class Comments(INIBasedModel.Comments):
)

_header: Literal["Sediment"] = "Sediment"
sedimentmodelnr: Optional[int] = Field(alias="Sedimentmodelnr")
sedimentmodelnr: Optional[int] = Field(0, alias="Sedimentmodelnr")
morfile: DiskOnlyFileModel = Field(
default_factory=lambda: DiskOnlyFileModel(None), alias="MorFile"
)
Expand Down Expand Up @@ -815,13 +815,13 @@ class Comments(INIBasedModel.Comments):
dtnodal: float = Field(21600.0, alias="dtNodal")
dtmax: float = Field(30.0, alias="dtMax")
dtinit: float = Field(1.0, alias="dtInit")
autotimestep: Optional[int] = Field(None, alias="autoTimestep")
autotimestep: Optional[int] = Field(1, alias="autoTimestep")
autotimestepnostruct: bool = Field(False, alias="autoTimestepNoStruct")
autotimestepnoqout: bool = Field(True, alias="autoTimestepNoQout")
tstart: float = Field(0.0, alias="tStart")
tstop: float = Field(86400.0, alias="tStop")
startdatetime: Optional[str] = Field(None, alias="startDateTime")
stopdatetime: Optional[str] = Field(None, alias="stopDateTime")
startdatetime: Optional[str] = Field("", alias="startDateTime")
stopdatetime: Optional[str] = Field("", alias="stopDateTime")
updateroughnessinterval: float = Field(86400.0, alias="updateRoughnessInterval")

@validator("startdatetime", "stopdatetime")
Expand Down Expand Up @@ -859,7 +859,7 @@ class Comments(INIBasedModel.Comments):
restartfile: DiskOnlyFileModel = Field(
default_factory=lambda: DiskOnlyFileModel(None), alias="restartFile"
)
restartdatetime: Optional[str] = Field(None, alias="restartDateTime")
restartdatetime: Optional[str] = Field("", alias="restartDateTime")

@validator("restartdatetime")
def _validate_datetime(cls, value, field):
Expand Down Expand Up @@ -973,10 +973,10 @@ class Comments(INIBasedModel.Comments):

_header: Literal["Trachytopes"] = "Trachytopes"
trtrou: str = Field("N", alias="trtRou") # TODO bool
trtdef: Optional[Path] = Field(None, alias="trtDef")
trtl: Optional[Path] = Field(None, alias="trtL")
trtdef: Optional[Path] = Field("", alias="trtDef")
trtl: Optional[Path] = Field("", alias="trtL")
dttrt: float = Field(60.0, alias="dtTrt")
trtmxr: Optional[int] = Field(None, alias="trtMxR")
trtmxr: Optional[int] = Field(8, alias="trtMxR")


ObsFile = Union[XYNModel, ObservationPointModel]
Expand Down Expand Up @@ -1492,8 +1492,8 @@ class Comments(INIBasedModel.Comments):
wrishp_enc: bool = Field(False, alias="wrishp_enc")
wrishp_src: bool = Field(False, alias="wrishp_src")
wrishp_pump: bool = Field(False, alias="wrishp_pump")
outputdir: Optional[Path] = Field(None, alias="outputDir")
waqoutputdir: Optional[Path] = Field(None, alias="waqOutputDir")
outputdir: Optional[Path] = Field("", alias="outputDir")
waqoutputdir: Optional[Path] = Field("", alias="waqOutputDir")
flowgeomfile: DiskOnlyFileModel = Field(
default_factory=lambda: DiskOnlyFileModel(None), alias="flowGeomFile"
)
Expand Down Expand Up @@ -1771,7 +1771,10 @@ class Comments(INIBasedModel.Comments):
partitionfile: Optional[str] = Field(
"<*_part.pol>, polyline(s) x, y.", alias="partitionFile"
)
uniformwidth1d: Optional[str] = Field(None, alias="uniformWidth1D")
uniformwidth1d: Optional[str] = Field(
"Uniform width for channel profiles not specified by profloc",
alias="uniformWidth1D",
)
dxwuimin2d: Optional[str] = Field(
"Smallest fraction dx/wu , set dx > Dxwuimin2D*wu",
alias="dxWuiMin2D",
Expand Down Expand Up @@ -2009,9 +2012,9 @@ class Comments(INIBasedModel.Comments):
numtopsig: int = Field(0, alias="numTopSig")
numtopsiguniform: bool = Field(True, alias="numTopSigUniform")
sigmagrowthfactor: float = Field(1.0, alias="sigmaGrowthFactor")
dztop: Optional[float] = Field(None, alias="dzTop")
floorlevtoplay: Optional[float] = Field(None, alias="floorLevTopLay")
dztopuniabovez: Optional[float] = Field(None, alias="dzTopUniAboveZ")
dztop: Optional[float] = Field(-999, alias="dzTop")
floorlevtoplay: Optional[float] = Field(-999, alias="floorLevTopLay")
dztopuniabovez: Optional[float] = Field(-999, alias="dzTopUniAboveZ")
keepzlayeringatbed: int = Field(2, alias="keepZLayeringAtBed")
dxdoubleat1dendnodes: bool = Field(True, alias="dxDoubleAt1DEndNodes")
changevelocityatstructures: bool = Field(False, alias="changeVelocityAtStructures")
Expand Down Expand Up @@ -2116,12 +2119,17 @@ class GroundWater(INIBasedModel):
"""

class Comments(INIBasedModel.Comments):
groundwater: Optional[str] = Field(None, alias="GroundWater")
groundwater: Optional[str] = Field(
"0=No (horizontal) groundwater flow, 1=With groundwater flow",
alias="GroundWater",
)
infiltrationmodel: Optional[str] = Field(
"Infiltration method (0: No infiltration, 1: Interception layer, 2: Constant infiltration capacity, 3: model unsaturated/saturated (with grw), 4: Horton).",
alias="Infiltrationmodel",
)
hinterceptionlayer: Optional[str] = Field(None, alias="Hinterceptionlayer")
hinterceptionlayer: Optional[str] = Field(
"Intercept this amount of rain (m)", alias="Hinterceptionlayer"
)
unifinfiltrationcapacity: Optional[str] = Field(
"Uniform maximum infiltration capacity [m/s].",
alias="UnifInfiltrationCapacity",
Expand Down Expand Up @@ -2153,15 +2161,15 @@ class Comments(INIBasedModel.Comments):
infiltrationmodel: Optional[InfiltrationMethod] = Field(
InfiltrationMethod.NoInfiltration, alias="Infiltrationmodel"
)
hinterceptionlayer: Optional[float] = Field(None, alias="Hinterceptionlayer")
hinterceptionlayer: Optional[float] = Field(0.0, alias="Hinterceptionlayer")
unifinfiltrationcapacity: Optional[float] = Field(
0.0, alias="UnifInfiltrationCapacity"
)
conductivity: Optional[float] = Field(0.0, alias="Conductivity")
h_aquiferuni: Optional[float] = Field(20.0, alias="h_aquiferuni")
bgrwuni: Optional[float] = Field(None, alias="bgrwuni")
bgrwuni: Optional[float] = Field(-999, alias="bgrwuni")
h_unsatini: Optional[float] = Field(0.2, alias="h_unsatini")
sgrwini: Optional[float] = Field(None, alias="sgrwini")
sgrwini: Optional[float] = Field(-999, alias="sgrwini")


class ProcessFluxIntegration(IntEnum):
Expand Down Expand Up @@ -2204,7 +2212,6 @@ class Comments(INIBasedModel.Comments):
"Waq processes time step [s]. Must be a multiple of DtUser. If DtProcesses is negative, water quality processes are calculated with every hydrodynamic time step.",
alias="DtProcesses",
)
dtmassbalance: Optional[str] = Field(None, alias="DtMassBalance")
processfluxintegration: Optional[str] = Field(
"Process fluxes integration option (1: WAQ, 2: D-Flow FM).",
alias="ProcessFluxIntegration",
Expand Down Expand Up @@ -2242,7 +2249,6 @@ class Comments(INIBasedModel.Comments):
)
thetavertical: Optional[float] = Field(0.0, alias="ThetaVertical")
dtprocesses: Optional[float] = Field(0.0, alias="DtProcesses")
dtmassbalance: Optional[float] = Field(0.0, alias="DtMassBalance")
processfluxintegration: Optional[ProcessFluxIntegration] = Field(
ProcessFluxIntegration.WAQ, alias="ProcessFluxIntegration"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,6 @@ AdditionalHistoryOutputFile = # extra history
StatisticsFile = # statistics file
ThetaVertical = 0. # theta vertical for waq
DtProcesses = 0. # waq processes time step
DtMassBalance = 0. # waq mass balance output time step
ProcessFluxIntegration = 1 # Process fluxes integration option (1: WAQ, 2: D-Flow FM)
Wriwaqbot3Doutput = 0 # Write 3D water quality bottom variables (1: yes, 0: no)
VolumeDryThreshold = 1.d-3
Expand Down
11 changes: 4 additions & 7 deletions tests/data/reference/fm/special_3d_settings.mdu
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ profDefFile = # <*_profdefinition.def>) def
profDefXyzFile = # <*_profdefinition.def>) definition for all profile nrs.
manholeFile = # File containing manholes (e.g. <*.dat>).
partitionFile = # <*_part.pol>, polyline(s) x, y.
uniformWidth1D = 2.0
uniformWidth1D = 2.0 # Uniform width for channel profiles not specified by profloc
dxWuiMin2D = 0.1 # Smallest fraction dx/wu , set dx > Dxwuimin2D*wu, Default = 0.1
waterLevIni = 0.0 # Initial water level at missing s0 values
bedLevUni = 5.0 # Uniform bed level used at missing z values if BedlevType > 2
Expand Down Expand Up @@ -194,9 +194,9 @@ secondaryFlow = 0 # Secondary flow (0: no, 1: yes)
betaSpiral = 0.0 # Weight factor of the spiral flow intensity on flow dispersion stresses (0d0 = disabled).

[Sediment]
Sedimentmodelnr = # Sediment model nr, (0=no, 1=Krone, 2=SvR2007, 3=E-H, 4=MorphologyModule).
MorFile = # Morphology settings file (*.mor)
SedFile = # Sediment characteristics file (*.sed)
Sedimentmodelnr = 0 # Sediment model nr, (0=no, 1=Krone, 2=SvR2007, 3=E-H, 4=MorphologyModule).
MorFile = # Morphology settings file (*.mor)
SedFile = # Sediment characteristics file (*.sed)

[Wind]
iCdTyp = 4 # Wind drag coefficient type (1=Const; 2=Smith&Banke (2 pts); 3=S&B (3 pts); 4=Charnock 1955, 5=Hwang 2005, 6=Wuest 2005, 7=Hersbach 2010 (2 pts)
Expand Down Expand Up @@ -234,9 +234,6 @@ restartDateTime = # Restart date and time (yyyymmddhhmmss) when restarting from
[External Forcing]
extForceFile = SEA_3D_3km_spatial.ext # Old format for external forcings file *.ext, link with tim/cmp-format boundary conditions specification
extForceFileNew = # New format for external forcings file *.ext, link with bc-format boundary conditions specification
rainfall = # Include rainfall, (0=no, 1=yes).
qExt = # Include user Qin/out, externally provided, (0=no, 1=yes).
evaporation = # Include evaporation in water balance, (0=no, 1=yes).
windExt = 1 # Include wind, externally provided, (0=no, 1=reserved for EC, 2=yes)

[Hydrology]
Expand Down
38 changes: 36 additions & 2 deletions tests/dflowfm/ini/test_ini.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import inspect
from itertools import chain
from typing import Iterable, List, Optional, Sequence, Tuple, Union
from typing import Iterable, List, Optional, Union

import pytest
from pydantic.v1 import ValidationError
from pydantic.v1 import Field, ValidationError

from hydrolib.core.basemodel import FileModel, ModelSaveSettings
from hydrolib.core.dflowfm.ini.io_models import (
CommentBlock,
ContentElement,
Expand All @@ -13,6 +14,7 @@
Property,
Section,
)
from hydrolib.core.dflowfm.ini.models import INIBasedModel
from hydrolib.core.dflowfm.ini.parser import (
Parser,
ParserConfig,
Expand Down Expand Up @@ -1696,6 +1698,38 @@ def test_deserialize_serialize_should_give_the_same_result(self):

assert result == input_str

def test_non_filemodel_keyword_with_none_value_does_not_get_added_to_section(self):
class TestINIBasedModel(INIBasedModel):
random_property: str = Field(None)

config = INISerializerConfig()
settings = ModelSaveSettings()
model = TestINIBasedModel()

section = model._to_section(config, settings)

assert len(section.content) == 0

def test_filemodel_keyword_with_none_value_does_get_added_to_section(self):
class TestINIBasedModel(INIBasedModel):
random_property: FileModel = Field(None)
random_property2: Union[FileModel, str] = Field(None)
random_property3: List[FileModel] = Field(None)

config = INISerializerConfig()
settings = ModelSaveSettings()
model = TestINIBasedModel()

section = model._to_section(config, settings)

assert len(section.content) == 3
assert section.content[0].key == "random_property"
assert section.content[0].value == ""
assert section.content[1].key == "random_property2"
assert section.content[1].value == ""
assert section.content[2].key == "random_property3"
assert section.content[2].value == ""


def test_serialize_deserialize_should_give_the_same_result():
document = Document(
Expand Down
1 change: 0 additions & 1 deletion tests/dflowfm/test_mdu.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ def test_mdu_with_optional_sections(self):
assert fm_model.processes.statisticsfile.filepath is None
assert fm_model.processes.thetavertical == 0.0
assert fm_model.processes.dtprocesses == 0.0
assert fm_model.processes.dtmassbalance == 0.0
assert fm_model.processes.processfluxintegration == ProcessFluxIntegration.WAQ
assert fm_model.processes.wriwaqbot3doutput is False
assert fm_model.processes.volumedrythreshold == 1e-3
Expand Down
28 changes: 28 additions & 0 deletions tests/dflowfm/test_research.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from pathlib import Path

import pytest

from hydrolib.core.dflowfm.research.models import (
Expand Down Expand Up @@ -51,3 +53,29 @@ def test_load_model_with_research_keywords_as_researchfmmodel(self):
assert model.time.research_dtfacmax == pytest.approx(1.1)
assert model.trachytopes.research_trtmnh == pytest.approx(0.1)
assert model.output.research_mbainterval == pytest.approx(0.0)

def test_save_model_with_single_research_keyword_does_not_write_other_research_keywords(
self, tmpdir: Path
):
model = ResearchFMModel()
model.geometry.research_waterdepthini1d = 12.34 # a random research keyword

save_path = tmpdir / "test.mdu"
model.save(filepath=save_path)

# I picked 5 random research keywords to check
keywords_to_check = [
"inputspecific",
"toplayminthick",
"faclaxturb",
"surftempsmofac",
"mbalumpsourcesinks",
]

with open(save_path, "r") as file:
content = file.read()

for keyword in keywords_to_check:
assert keyword not in content

assert "waterdepthini1d" in content