From 780198eff247204b9bc54a0dd440a5ba18c38132 Mon Sep 17 00:00:00 2001 From: David Almeida Date: Wed, 9 Oct 2024 10:42:12 +0200 Subject: [PATCH 1/5] Expand stray tag check to all attributes --- nomenclature/codelist.py | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/nomenclature/codelist.py b/nomenclature/codelist.py index c488df6f..445bd603 100644 --- a/nomenclature/codelist.py +++ b/nomenclature/codelist.py @@ -46,13 +46,28 @@ 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." - ) + """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 codelist: {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") From f7620d598b5ad5b9a855693457fd7df61c9d5dc2 Mon Sep 17 00:00:00 2001 From: David Almeida Date: Wed, 9 Oct 2024 10:42:39 +0200 Subject: [PATCH 2/5] Update stray tag tests --- .../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 | 18 +++++++++++++++--- 5 files changed, 43 insertions(+), 4 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/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..ee12f35e --- /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: EJ/yr 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..ea00b19f --- /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: EJ/yr + 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..e34cf239 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: EJ/yr diff --git a/tests/test_codelist.py b/tests/test_codelist.py index 885efd1f..e7a51e43 100644 --- a/tests/test_codelist.py +++ b/tests/test_codelist.py @@ -213,12 +213,24 @@ def test_to_csv(): def test_stray_tag_fails(): - """Check that typos in a tag raises expected error""" + """Check that stray brackets from, e.g. typos in a tag, raises expected error""" + + match = r"Unexpected bracket in codelist: Primary Energy\|{Feul}" + with raises(ValueError, match=match): + VariableCodeList.from_directory( + "variable", MODULE_TEST_DATA_DIR / "stray_tag" / "char_in_str" + ) + + match = r"Unexpected bracket in codelist: Share\|Coal" + with raises(ValueError, match=match): + VariableCodeList.from_directory( + "variable", MODULE_TEST_DATA_DIR / "stray_tag" / "char_in_list" + ) - match = r"Unexpected {} in codelist: Primary Energy\|{Feul}" + match = r"Unexpected bracket in codelist: Primary Energy" with raises(ValueError, match=match): VariableCodeList.from_directory( - "variable", MODULE_TEST_DATA_DIR / "stray_tag" / "definitions" / "variable" + "variable", MODULE_TEST_DATA_DIR / "stray_tag" / "char_in_dict" ) From a94770f192f7e748bed8f4d55d1ecc74b5b427f5 Mon Sep 17 00:00:00 2001 From: David Almeida Date: Wed, 9 Oct 2024 15:33:20 +0200 Subject: [PATCH 3/5] Parametrize test --- tests/test_codelist.py | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/tests/test_codelist.py b/tests/test_codelist.py index e7a51e43..074af394 100644 --- a/tests/test_codelist.py +++ b/tests/test_codelist.py @@ -212,25 +212,19 @@ def test_to_csv(): assert obs == exp -def test_stray_tag_fails(): +@pytest.mark.parametrize( + "subfolder, match", + [ + ("char_in_str", r"Unexpected bracket in codelist: Primary Energy\|{Feul}"), + ("char_in_list", r"Unexpected bracket in codelist: Share\|Coal"), + ("char_in_dict", r"Unexpected bracket in codelist: Primary Energy"), + ], +) +def test_stray_tag_fails(subfolder, match): """Check that stray brackets from, e.g. typos in a tag, raises expected error""" - - match = r"Unexpected bracket in codelist: Primary Energy\|{Feul}" - with raises(ValueError, match=match): - VariableCodeList.from_directory( - "variable", MODULE_TEST_DATA_DIR / "stray_tag" / "char_in_str" - ) - - match = r"Unexpected bracket in codelist: Share\|Coal" - with raises(ValueError, match=match): - VariableCodeList.from_directory( - "variable", MODULE_TEST_DATA_DIR / "stray_tag" / "char_in_list" - ) - - match = r"Unexpected bracket in codelist: Primary Energy" with raises(ValueError, match=match): VariableCodeList.from_directory( - "variable", MODULE_TEST_DATA_DIR / "stray_tag" / "char_in_dict" + "variable", MODULE_TEST_DATA_DIR / "stray_tag" / subfolder ) From c32f6f50b2deb920a4b115f305b6077b555df56b Mon Sep 17 00:00:00 2001 From: David Almeida Date: Wed, 9 Oct 2024 15:33:51 +0200 Subject: [PATCH 4/5] Fix variable unit --- tests/data/codelist/stray_tag/char_in_dict/variables.yaml | 2 +- tests/data/codelist/stray_tag/char_in_list/variables.yaml | 2 +- tests/data/codelist/stray_tag/char_in_str/variables.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/data/codelist/stray_tag/char_in_dict/variables.yaml b/tests/data/codelist/stray_tag/char_in_dict/variables.yaml index ee12f35e..422ffa24 100644 --- a/tests/data/codelist/stray_tag/char_in_dict/variables.yaml +++ b/tests/data/codelist/stray_tag/char_in_dict/variables.yaml @@ -10,4 +10,4 @@ unit: EJ/yr - Share|Coal: definition: Share of coal in the total primary energy mix - unit: EJ/yr + 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 index ea00b19f..00672726 100644 --- a/tests/data/codelist/stray_tag/char_in_list/variables.yaml +++ b/tests/data/codelist/stray_tag/char_in_list/variables.yaml @@ -6,7 +6,7 @@ unit: EJ/yr - Share|Coal: definition: Share of coal in the total primary energy mix - unit: EJ/yr + unit: '%' info: - Valid information. - Invalid bracket { information. diff --git a/tests/data/codelist/stray_tag/char_in_str/variables.yaml b/tests/data/codelist/stray_tag/char_in_str/variables.yaml index e34cf239..175cd4d4 100644 --- a/tests/data/codelist/stray_tag/char_in_str/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: EJ/yr + unit: '%' From eae0ac4a33a4b43ea678fa154e50c91cb3bfc28c Mon Sep 17 00:00:00 2001 From: David Almeida Date: Wed, 9 Oct 2024 16:35:32 +0200 Subject: [PATCH 5/5] Improve error message --- nomenclature/codelist.py | 6 ++++-- tests/test_codelist.py | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/nomenclature/codelist.py b/nomenclature/codelist.py index 445bd603..1233a2a4 100644 --- a/nomenclature/codelist.py +++ b/nomenclature/codelist.py @@ -45,7 +45,9 @@ def __eq__(self, other): @field_validator("mapping") @classmethod - def check_stray_tag(cls, v: Dict[str, Code]) -> Dict[str, Code]: + 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 = ["{", "}"] @@ -53,7 +55,7 @@ def _check_string(value): if isinstance(value, str): if any(char in value for char in forbidden): raise ValueError( - f"Unexpected bracket in codelist: {code.name}." + f"Unexpected bracket in {info.data['name']}: '{code.name}'." " Check if tags were spelled correctly." ) elif isinstance(value, dict): diff --git a/tests/test_codelist.py b/tests/test_codelist.py index 074af394..0ddca72d 100644 --- a/tests/test_codelist.py +++ b/tests/test_codelist.py @@ -215,9 +215,9 @@ def test_to_csv(): @pytest.mark.parametrize( "subfolder, match", [ - ("char_in_str", r"Unexpected bracket in codelist: Primary Energy\|{Feul}"), - ("char_in_list", r"Unexpected bracket in codelist: Share\|Coal"), - ("char_in_dict", r"Unexpected bracket in codelist: Primary Energy"), + ("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):