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: Raise error for unknown keywords #632

Merged
merged 33 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
4c64a2d
feat: (622) write message on reading file and setting variables based…
MRVermeulenDeltares Apr 24, 2024
4ee0250
feat: (622) resolve failing tests.
MRVermeulenDeltares Apr 24, 2024
ca14542
autoformat: isort & black
MRVermeulenDeltares Apr 24, 2024
0808f17
feat: (622) move related changes to specific class UnknownKeyNotifica…
MRVermeulenDeltares Apr 24, 2024
5980d31
feat: (622) Move UnknownKeyNotificationManager to util
MRVermeulenDeltares Apr 24, 2024
49ec80f
feat: (622) Add unit tests for UnknownKeyNotificationManager
MRVermeulenDeltares Apr 24, 2024
9ae839e
feat: (622) Add extra spacing at the end of printing the list per sec…
MRVermeulenDeltares Apr 24, 2024
969ee20
feat: (622) Try to resolve failing tests on python 3.8
MRVermeulenDeltares Apr 24, 2024
2cae2a9
Revert "feat: (622) Try to resolve failing tests on python 3.8"
MRVermeulenDeltares Apr 24, 2024
b6f2ebc
feat: (622) Resolve problem occuring in python 3.8
MRVermeulenDeltares Apr 24, 2024
060539c
autoformat: isort & black
MRVermeulenDeltares Apr 24, 2024
790dfe4
feat: (622) Update unknown keyword manager to raise an error instead …
MRVermeulenDeltares May 10, 2024
4ee0166
Merge branch 'main' into feat/622-give-warning-for-unknown-keywords
MRVermeulenDeltares May 10, 2024
987564d
feat: (622) implement review suggestion and remove unused imports.
MRVermeulenDeltares May 27, 2024
741cba7
feat: (622) Update tests and rename tests
MRVermeulenDeltares May 27, 2024
19b317d
feat: (622) add review suggestion check on alias too.
MRVermeulenDeltares May 27, 2024
712f757
feat: (622) remove unknown keys from mdu files and isshared from crs …
MRVermeulenDeltares May 28, 2024
3c3894f
feat: (622) resolve failing tests.
MRVermeulenDeltares May 28, 2024
c4e3483
autoformat: isort & black
MRVermeulenDeltares May 28, 2024
88c872f
feat: (622) resolve code smells.
MRVermeulenDeltares May 28, 2024
c1eb708
feat: (622) make a MduUnknownKeywordErrorManager for only checking on…
MRVermeulenDeltares May 28, 2024
4f84652
feat: (622) revert structure test cases.
MRVermeulenDeltares May 28, 2024
d247030
feat: (622) revert crs def ini file changes.
MRVermeulenDeltares May 28, 2024
5a368a4
Update CrossSectionDefinitions.ini
MRVermeulenDeltares May 28, 2024
4e04665
Update CrossSectionDefinitions.ini
MRVermeulenDeltares May 28, 2024
719c412
Update CrossSectionDefinitions.ini
MRVermeulenDeltares May 28, 2024
5ce7b27
autoformat: isort & black
MRVermeulenDeltares May 28, 2024
9fe75b6
feat: (622) revert crs def ini file changes.
MRVermeulenDeltares May 28, 2024
7ed3f01
feat: (622) revert structure test cases.
MRVermeulenDeltares May 28, 2024
1a77689
feat: (622) Pair program to implement root_validator
MRVermeulenDeltares May 30, 2024
57d10e3
autoformat: isort & black
MRVermeulenDeltares May 30, 2024
f007066
feat: (622) add review suggestions typehinting and documentation.
MRVermeulenDeltares May 30, 2024
5897d05
Merge branch 'main' into feat/622-give-warning-for-unknown-keywords
tim-vd-aardweg Jun 7, 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
14 changes: 12 additions & 2 deletions hydrolib/core/dflowfm/ini/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
INISerializerConfig,
write_ini,
)
from .util import make_list_validator
from .util import UnknownKeywordErrorManager, make_list_validator

logger = logging.getLogger(__name__)

Expand All @@ -45,8 +45,9 @@ class INIBasedModel(BaseModel, ABC):
descriptions for all data fields.
"""

_header: str
_header: str = ""
_file_path_style_converter = FilePathStyleConverter()
_unknown_keyword_error_manager = UnknownKeywordErrorManager()
_scientific_notation_regex = compile(
r"([\d.]+)([dD])([+-]?\d{1,3})"
) # matches a float: 1d9, 1D-3, 1.D+4, etc.
Expand All @@ -55,6 +56,15 @@ class Config:
extra = Extra.ignore
arbitrary_types_allowed = False

def __init__(self, **data):
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
super().__init__(**data)
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
self._unknown_keyword_error_manager.raise_error_for_unknown_keywords(
data,
MRVermeulenDeltares marked this conversation as resolved.
Show resolved Hide resolved
self._header,
self.__fields__,
self._exclude_fields(),
)

@classmethod
def _supports_comments(cls):
return True
Expand Down
48 changes: 47 additions & 1 deletion hydrolib/core/dflowfm/ini/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
from datetime import datetime
from enum import Enum
from operator import eq
from typing import Any, Callable, Dict, List, Optional, Type
from typing import Any, Callable, Dict, List, Optional, Set, Type

from pydantic import Extra
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
from pydantic.v1.class_validators import root_validator, validator
from pydantic.v1.fields import ModelField
from pydantic.v1.main import BaseModel
Expand Down Expand Up @@ -622,3 +623,48 @@ def rename_keys_for_backwards_compatibility(
break

return values


class UnknownKeywordErrorManager:
"""
Error manager for unknown keys.
Detects unknown keys and manages the Error to the user.
"""

def raise_error_for_unknown_keywords(
self,
data: Dict[str, Any],
section_header: str,
fields: Dict[str, Any],
excluded_fields: Set,
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
):
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
"""
Notify the user of unknown keywords.

Args:
data (Dict[str, Any]) : Input data containing all set properties which are checked on unknown keywords.
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
section_header (str) : Header of the section in which unknown keys might be detected.
fields (Dict[str, Any]) : Known fields of the section.
excluded_fields (Set) : Fields which should be excluded from the check for unknown keywords.
"""
unknown_keywords = self._get_all_unknown_keywords(data, fields, excluded_fields)
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved

if len(unknown_keywords) == 0:
return

raise ValueError(f"Unknown keywords are detected in section: '{section_header}', '{unknown_keywords}'")

def _get_all_unknown_keywords(
self, data: Dict[str, Any], fields: Dict[str, Any], excluded_fields: Set
) -> List[str]:
list_of_unknown_keywords = []
for name, _ in data.items():
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
if self._is_unknown_keyword(name, fields, excluded_fields):
list_of_unknown_keywords.append(name)

return list_of_unknown_keywords

def _is_unknown_keyword(
self, name: str, fields: Dict[str, Any], excluded_fields: Set
):
return name not in fields and name not in excluded_fields
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
25 changes: 25 additions & 0 deletions tests/dflowfm/ini/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from hydrolib.core.dflowfm.ini.util import (
LocationValidationConfiguration,
LocationValidationFieldNames,
UnknownKeywordErrorManager,
get_from_subclass_defaults,
get_type_based_on_subclass_default_value,
rename_keys_for_backwards_compatibility,
Expand Down Expand Up @@ -356,3 +357,27 @@ class WithoutDefaultProperty(BaseClass):

class GrandChildWithDefaultProperty(WithoutDefaultProperty):
name: Literal["GrandChildWithDefaultProperty"] = "GrandChildWithDefaultProperty"


class TestUnknownKeywordErrorManager:

def test_unknown_keywords_given_when_notify_unknown_keywords_gives_error_with_unknown_keywords(
self
):
section_header = "section header"
fields = {}
excluded_fields = set()
name = "keyname"
second_name = "second_other"

ukem = UnknownKeywordErrorManager()
data = {name: 1, second_name: 2}

expected_message = f"Unknown keywords are detected in section: '{section_header}', '{[name, second_name]}'"

with pytest.raises(ValueError) as exc_err:
ukem.raise_error_for_unknown_keywords(
data, section_header, fields, excluded_fields
)

assert expected_message in str(exc_err.value)
116 changes: 116 additions & 0 deletions tests/dflowfm/test_mdu.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
from pathlib import Path
from unittest import mock
from unittest.mock import patch

from pydantic.v1.config import Extra

from hydrolib.core.basemodel import DiskOnlyFileModel
from hydrolib.core.dflowfm.mdu.models import (
Expand Down Expand Up @@ -388,3 +392,115 @@ def test_loading_fmmodel_model_with_both_ini_and_xyn_obsfiles(self):
assert actual_point.x == expected_point.x
assert actual_point.y == expected_point.y
assert actual_point.name == expected_point.name

def test_mdu_unknown_keywords_loading_gives_message_for_missing_keyword(
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
self, tmp_path, capsys
):
tmp_mdu = """
[General]
fileVersion = 1.09
fileType = modelDef
program = D-Flow FM
version = 1.2.100.66357
autoStart = 0
pathsRelativeToParent = 0
unknownkey = something
"""

tmp_mdu_path = tmp_path / "tmp.mdu"
tmp_mdu_path.write_text(tmp_mdu)

with patch(
"hydrolib.core.dflowfm.ini.models.INIBasedModel.Config"
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
) as mock_config:
mock_config.extra = Extra.ignore

FMModel(filepath=tmp_mdu_path)
captured = capsys.readouterr()

section = "General"
name = "unknownkey"

assert (
f"Unknown keywords are detected in '{section}', these keywords will be dropped:"
in captured.out
)
assert name in captured.out

def test_mdu_unknown_keywords_loading_gives_message_for_missing_keyword2(
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
self, tmp_path, capsys
):
tmp_mdu = """
[General]
fileVersion = 1.09
fileType = modelDef
program = D-Flow FM
version = 1.2.100.66357
autoStart = 0
pathsRelativeToParent = 0
unknownkey = something
"""

tmp_mdu_path = tmp_path / "tmp.mdu"
tmp_mdu_path.write_text(tmp_mdu)

with patch(
"hydrolib.core.dflowfm.ini.models.INIBasedModel.Config"
) as mock_config:
mock_config.extra = Extra.allow

FMModel(filepath=tmp_mdu_path)
captured = capsys.readouterr()

section = "General"
name = "unknownkey"

assert (
f"Unknown keywords are detected in '{section}', these keywords will be kept in memory but will have no validation:"
in captured.out
)
assert name in captured.out

def test_mdu_unknown_keywords_loading_does_not_give_message_on_exclude_fields(
self, tmp_path, capsys
):
tmp_mdu = """
[General]
fileVersion = 1.09
fileType = modelDef
program = D-Flow FM
version = 1.2.100.66357
autoStart = 0
pathsRelativeToParent = 0
unknownkey = something
"""

tmp_mdu_path = tmp_path / "tmp.mdu"
tmp_mdu_path.write_text(tmp_mdu)

FMModel(filepath=tmp_mdu_path)
captured = capsys.readouterr()

excluded_fields = ["comments", "datablock", "_header"]
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
for excluded_field in excluded_fields:
assert excluded_field not in captured.out

def test_mdu_unknown_keywords_allow_extra_setting_field_gives_message(self, capsys):
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
model = FMModel()
config_extra_setting = (
model.general.Config.extra
) # Save the Config.extra to revert back to original setting after the test.

model.general.Config.extra = Extra.allow
model.general.unknown = "something"
captured = capsys.readouterr()

model.general.Config.extra = config_extra_setting # Revert the Config.extra to the original setting, if this is not done it can affect other tests.

section = "General"
name = "unknown"

assert (
f"Unknown keyword detected in '{section}', '{name}', keyword will be kept in memory but will have no validation."
in captured.out
)
Loading