From e64ed34d5a18c4b3e9f9715f1d51e193159d4676 Mon Sep 17 00:00:00 2001 From: David Almeida <58078834+dc-almeida@users.noreply.github.com> Date: Wed, 9 Oct 2024 16:40:10 +0200 Subject: [PATCH] Extend stray tag check (#411) * Expand stray tag check to all attributes * Update stray tag tests * Parametrize test * Fix variable unit * Improve error message --- nomenclature/codelist.py | 33 ++++++++++++++----- .../stray_tag/char_in_dict/variables.yaml | 13 ++++++++ .../stray_tag/char_in_list/variables.yaml | 14 ++++++++ .../variable => char_in_str}/tag_fuel.yaml | 0 .../variable => char_in_str}/variables.yaml | 2 +- tests/test_codelist.py | 16 ++++++--- 6 files changed, 64 insertions(+), 14 deletions(-) create mode 100644 tests/data/codelist/stray_tag/char_in_dict/variables.yaml create mode 100644 tests/data/codelist/stray_tag/char_in_list/variables.yaml rename tests/data/codelist/stray_tag/{definitions/variable => char_in_str}/tag_fuel.yaml (100%) rename tests/data/codelist/stray_tag/{definitions/variable => char_in_str}/variables.yaml (94%) diff --git a/nomenclature/codelist.py b/nomenclature/codelist.py index c488df6f..1233a2a4 100644 --- a/nomenclature/codelist.py +++ b/nomenclature/codelist.py @@ -45,14 +45,31 @@ def __eq__(self, other): @field_validator("mapping") @classmethod - def check_stray_tag(cls, v: Dict[str, Code]) -> Dict[str, Code]: - """Check that no '{' are left in codes after tag replacement""" - for code in v: - if "{" in code: - raise ValueError( - f"Unexpected {{}} in codelist: {code}." - " Check if the tag was spelled correctly." - ) + def check_stray_tag( + cls, v: Dict[str, Code], info: ValidationInfo + ) -> Dict[str, Code]: + """Check that no stray tags are left in codes after tag replacement""" + forbidden = ["{", "}"] + + def _check_string(value): + if isinstance(value, str): + if any(char in value for char in forbidden): + raise ValueError( + f"Unexpected bracket in {info.data['name']}: '{code.name}'." + " Check if tags were spelled correctly." + ) + elif isinstance(value, dict): + for v in value.values(): + _check_string(v) + elif isinstance(value, list): + for item in value: + _check_string(item) + + for code in v.values(): + for attr in code.model_fields: + if attr not in ["file", "repository"]: + value = getattr(code, attr) + _check_string(value) return v @field_validator("mapping") diff --git a/tests/data/codelist/stray_tag/char_in_dict/variables.yaml b/tests/data/codelist/stray_tag/char_in_dict/variables.yaml new file mode 100644 index 00000000..422ffa24 --- /dev/null +++ b/tests/data/codelist/stray_tag/char_in_dict/variables.yaml @@ -0,0 +1,13 @@ +- Primary Energy: + definition: Total primary energy consumption + unit: EJ/yr + info: + valid: Valid information. + invalid: Invalid bracket } information. + final: Another valid information. +- Primary Energy|Coal: + definition: Primary energy consumption of coal + unit: EJ/yr +- Share|Coal: + definition: Share of coal in the total primary energy mix + unit: '%' diff --git a/tests/data/codelist/stray_tag/char_in_list/variables.yaml b/tests/data/codelist/stray_tag/char_in_list/variables.yaml new file mode 100644 index 00000000..00672726 --- /dev/null +++ b/tests/data/codelist/stray_tag/char_in_list/variables.yaml @@ -0,0 +1,14 @@ +- Primary Energy: + definition: Total primary energy consumption + unit: EJ/yr +- Primary Energy|Coal: + definition: Primary energy consumption of coal + unit: EJ/yr +- Share|Coal: + definition: Share of coal in the total primary energy mix + unit: '%' + info: + - Valid information. + - Invalid bracket { information. + - Another valid information. + diff --git a/tests/data/codelist/stray_tag/definitions/variable/tag_fuel.yaml b/tests/data/codelist/stray_tag/char_in_str/tag_fuel.yaml similarity index 100% rename from tests/data/codelist/stray_tag/definitions/variable/tag_fuel.yaml rename to tests/data/codelist/stray_tag/char_in_str/tag_fuel.yaml diff --git a/tests/data/codelist/stray_tag/definitions/variable/variables.yaml b/tests/data/codelist/stray_tag/char_in_str/variables.yaml similarity index 94% rename from tests/data/codelist/stray_tag/definitions/variable/variables.yaml rename to tests/data/codelist/stray_tag/char_in_str/variables.yaml index 9bc24554..175cd4d4 100644 --- a/tests/data/codelist/stray_tag/definitions/variable/variables.yaml +++ b/tests/data/codelist/stray_tag/char_in_str/variables.yaml @@ -6,4 +6,4 @@ unit: EJ/yr - Share|{Fuel}: definition: Share of {Fuel} in the total primary energy mix - unit: + unit: '%' diff --git a/tests/test_codelist.py b/tests/test_codelist.py index 885efd1f..0ddca72d 100644 --- a/tests/test_codelist.py +++ b/tests/test_codelist.py @@ -212,13 +212,19 @@ def test_to_csv(): assert obs == exp -def test_stray_tag_fails(): - """Check that typos in a tag raises expected error""" - - match = r"Unexpected {} in codelist: Primary Energy\|{Feul}" +@pytest.mark.parametrize( + "subfolder, match", + [ + ("char_in_str", r"Unexpected bracket in variable: 'Primary Energy\|{Feul}'"), + ("char_in_list", r"Unexpected bracket in variable: 'Share\|Coal'"), + ("char_in_dict", r"Unexpected bracket in variable: 'Primary Energy'"), + ], +) +def test_stray_tag_fails(subfolder, match): + """Check that stray brackets from, e.g. typos in a tag, raises expected error""" with raises(ValueError, match=match): VariableCodeList.from_directory( - "variable", MODULE_TEST_DATA_DIR / "stray_tag" / "definitions" / "variable" + "variable", MODULE_TEST_DATA_DIR / "stray_tag" / subfolder )