Repository: IAMconsortium/nomenclature

Issue:
Check native regions belong to the same model in mappings
Following up on [this common-definitions PR](https://github.com/IAMconsortium/common-definitions/pull/247), check if the constituent regions used are the same native regions listed in the mappings file

Discussion:
dc-almeida: The two tests failing are caused by the common-definitions hash pointing to a version where the error found [here](https://github.com/IAMconsortium/common-definitions/pull/247) remains:
https://github.com/dc-almeida/nomenclature/blob/04cf36a2fd31407b3be6329c51150a981daac6ef/tests/data/region_processing/external_repo_test/nomenclature.yaml#L4-L7
phackstock: @dc-almeida, yes please go ahead and update the hash.

Patch:
diff --git a/nomenclature/codelist.py b/nomenclature/codelist.py
index 0b7f434..af4ff77 100644
--- a/nomenclature/codelist.py
+++ b/nomenclature/codelist.py
@@ -339,28 +339,27 @@ class CodeList(BaseModel):
         illegal = ["{", "}"] + config.illegal_characters
         errors = ErrorCollector()
 
-        def _check_string(value):
+        def _check_string(attr, value):
             if isinstance(value, str):
-                if any(char in value for char in illegal):
+                if found := set(illegal).intersection(value):
+                    found = "', '".join(sorted(found))
                     errors.append(
                         ValueError(
-                            f"Unexpected character in {self.name}: '{code.name}'."
-                            " Check for illegal characters and/or if tags were spelled correctly."
+                            f"Illegal character(s) '{found}' in {attr} of {self.name} '{code.name}'."
                         )
                     )
             elif isinstance(value, dict):
-                for k in value.keys():
-                    _check_string(k)
-                for v in value.values():
-                    _check_string(v)
+                for k, v in value.items():
+                    _check_string(k, k)
+                    _check_string(k, v)
             elif isinstance(value, list):
                 for item in value:
-                    _check_string(item)
+                    _check_string(attr, item)
 
         for code in self.mapping.values():
-            if not code.repository:
-                for value in code.model_dump(exclude="file").values():
-                    _check_string(value)
+            if not code.from_external_repository:
+                for attr, value in code.model_dump(exclude="file").items():
+                    _check_string(attr, value)
         if errors:
             raise ValueError(errors)
 
diff --git a/nomenclature/error.py b/nomenclature/error.py
index 4cdca95..958f3c3 100644
--- a/nomenclature/error.py
+++ b/nomenclature/error.py
@@ -40,6 +40,10 @@ pydantic_custom_error_config = {
         "region_not_defined",
         "Region(s)\n{regions}\nin {file}\nnot found in RegionCodeList",
     ),
+    "ConstituentsNotNativeError": (
+        "constituents_not_native",
+        "Constituent region(s)\n{regions}\nin {file} not found in native region(s)",
+    ),
 }
 
 PydanticCustomErrors = namedtuple("PydanticCustomErrors", pydantic_custom_error_config)
diff --git a/nomenclature/processor/data_validator.py b/nomenclature/processor/data_validator.py
index debf0e1..dda02f9 100644
--- a/nomenclature/processor/data_validator.py
+++ b/nomenclature/processor/data_validator.py
@@ -4,6 +4,7 @@ from enum import Enum
 from pathlib import Path
 
 import yaml
+from pandas import concat
 from pyam import IamDataFrame
 from pyam.logging import adjust_log_level
 from pydantic import computed_field, field_validator, model_validator
@@ -18,10 +19,10 @@ logger = logging.getLogger(__name__)
 
 
 class WarningEnum(str, Enum):
+    error = "error"
     high = "high"
     medium = "medium"
     low = "low"
-    error = "error"
 
 
 class DataValidationCriteria(IamcDataFilter):
@@ -47,6 +48,7 @@ class DataValidationCriteriaValue(DataValidationCriteria):
 
     @property
     def validation_args(self):
+        """Attributes used for validation (as bounds)."""
         return self.model_dump(
             exclude_none=True,
             exclude_unset=True,
@@ -55,6 +57,7 @@ class DataValidationCriteriaValue(DataValidationCriteria):
 
     @property
     def criteria(self):
+        """Attributes used for validation (as specified in the file)."""
         return self.model_dump(
             exclude_none=True,
             exclude_unset=True,
@@ -79,23 +82,46 @@ class DataValidationCriteriaBounds(DataValidationCriteria):
     @property
     def criteria(self):
         return self.model_dump(
-            exclude_none=True,
-            exclude_unset=True,
-            exclude=["warning_level"],
+            exclude_none=True, exclude_unset=True, exclude=["warning_level"]
+        )
+
+
+class DataValidationCriteriaMultiple(IamcDataFilter):
+    validation: (
+        list[DataValidationCriteriaValue | DataValidationCriteriaBounds] | None
+    ) = None
+
+    @model_validator(mode="after")
+    def check_warnings_order(self):
+        """Check if warnings are set in descending order of severity."""
+        if self.validation != sorted(self.validation, key=lambda c: c.warning_level):
+            raise ValueError(
+                f"Validation criteria for {self.criteria} not"
+                " in descending order of severity."
+            )
+        else:
+            return self
+
+    @property
+    def criteria(self):
+        """Attributes used for validation (as specified in the file)."""
+        return self.model_dump(
+            exclude_none=True, exclude_unset=True, exclude=["validation"]
         )
 
 
 class DataValidator(Processor):
     """Processor for validating IAMC datapoints"""
 
-    criteria_items: list[DataValidationCriteriaBounds | DataValidationCriteriaValue]
+    criteria_items: list[DataValidationCriteriaMultiple]
     file: Path
 
     @field_validator("criteria_items", mode="before")
     def check_criteria(cls, v):
-        for criterion in v:
-            has_bounds = any(c in criterion for c in ["upper_bound", "lower_bound"])
-            has_values = any(c in criterion for c in ["value", "atol", "rtol"])
+        for item in v:
+            for criterion in item["validation"]:
+                has_bounds = any(c in criterion for c in ["upper_bound", "lower_bound"])
+                has_values = any(c in criterion for c in ["value", "atol", "rtol"])
             if has_bounds and has_values:
                 raise ValueError(
                     f"Cannot use bounds and value-criteria simultaneously: {criterion}"
@@ -106,7 +132,21 @@ class DataValidator(Processor):
     def from_file(cls, file: Path | str) -> "DataValidator":
         with open(file, "r", encoding="utf-8") as f:
             content = yaml.safe_load(f)
-        return cls(file=file, criteria_items=content)
+        criteria_items = []
+        for item in content:
+            filter_args = {k: item[k] for k in item if k in IamcDataFilter.model_fields}
+            criteria_args = {
+                k: item[k]
+                for k in item
+                if k not in IamcDataFilter.model_fields and k != "validation"
+            }
+            if "validation" in item:
+                for criterion in item["validation"]:
+                    criterion.update(filter_args)
+            else:
+                item["validation"] = [{**filter_args, **criteria_args}]
+            criteria_items.append({k: item[k] for k in item if k not in criteria_args})
+        return cls(file=file, criteria_items=criteria_items)
 
     def apply(self, df: IamDataFrame) -> IamDataFrame:
         fail_list = []
@@ -114,18 +154,33 @@ class DataValidator(Processor):
 
         with adjust_log_level():
             for item in self.criteria_items:
-                failed_validation = df.validate(**item.validation_args)
-                if failed_validation is not None:
-                    criteria_msg = "  Criteria: " + ", ".join(
-                        [f"{key}: {value}" for key, value in item.criteria.items()]
-                    )
-                    failed_validation["warning_level"] = item.warning_level.value
-                    if item.warning_level == WarningEnum.error:
-                        error = True
-                    fail_list.append(criteria_msg)
-                    fail_list.append(
-                        textwrap.indent(str(failed_validation), prefix="    ") + "\n"
+                per_item_df = df
+                for criterion in item.validation:
+                    failed_validation = per_item_df.validate(
+                        **criterion.validation_args
                     )
+                    if failed_validation is not None:
+                        per_item_df = IamDataFrame(
+                            concat([df.data, failed_validation]).drop_duplicates(
+                                keep=False
+                            )
+                        )
+                        criteria_msg = "  Criteria: " + ", ".join(
+                            [
+                                f"{key}: {value}"
+                                for key, value in criterion.criteria.items()
+                            ]
+                        )
+                        failed_validation["warning_level"] = (
+                            criterion.warning_level.value
+                        )
+                        if criterion.warning_level == WarningEnum.error:
+                            error = True
+                        fail_list.append(criteria_msg)
+                        fail_list.append(
+                            textwrap.indent(str(failed_validation), prefix="    ")
+                            + "\n"
+                        )
             fail_msg = "(file %s):\n" % get_relative_path(self.file)
             if error:
                 fail_msg = (
diff --git a/nomenclature/processor/region.py b/nomenclature/processor/region.py
index c325583..552d978 100644
--- a/nomenclature/processor/region.py
+++ b/nomenclature/processor/region.py
@@ -220,6 +220,21 @@ class RegionAggregationMapping(BaseModel):
     ) -> "RegionAggregationMapping":
         return _check_exclude_region_overlap(v, "common_regions")
 
+    @model_validator(mode="after")
+    @classmethod
+    def check_constituent_regions_in_native_regions(
+        cls, v: "RegionAggregationMapping"
+    ) -> "RegionAggregationMapping":
+        if v.common_regions and v.native_regions:
+            if missing := set(
+                [cr for r in v.common_regions for cr in r.constituent_regions]
+            ).difference([r.name for r in v.native_regions] + v.exclude_regions):
+                raise PydanticCustomError(
+                    *custom_pydantic_errors.ConstituentsNotNativeError,
+                    {"regions": missing, "file": v.file},
+                )
+        return v
+
     @classmethod
     def from_file(cls, file: Path | str) -> "RegionAggregationMapping":
         """Initialize a RegionAggregationMapping from a file.


Test patch:
diff --git a/tests/data/cli/structure_validation_fails/mappings/mapping_2.yaml b/tests/data/cli/structure_validation_fails/mappings/mapping_2.yaml
index 1c171c6..be91833 100644
--- a/tests/data/cli/structure_validation_fails/mappings/mapping_2.yaml
+++ b/tests/data/cli/structure_validation_fails/mappings/mapping_2.yaml
@@ -4,4 +4,3 @@ native_regions:
 common_regions:
   - World:
     - region_a
-    - region_b
diff --git a/tests/data/region_processing/external_repo_test/nomenclature.yaml b/tests/data/region_processing/external_repo_test/nomenclature.yaml
index aae2198..4cf4f11 100644
--- a/tests/data/region_processing/external_repo_test/nomenclature.yaml
+++ b/tests/data/region_processing/external_repo_test/nomenclature.yaml
@@ -4,7 +4,7 @@ dimensions:
 repositories:
   common-definitions:
     url: https://github.com/IAMconsortium/common-definitions.git/
-    hash: cb85704
+    hash: 091c0fe
 definitions:
   region:
     repository: common-definitions
diff --git a/tests/data/region_processing/external_repo_test_missing_region/nomenclature.yaml b/tests/data/region_processing/external_repo_test_missing_region/nomenclature.yaml
index 1dacf52..b0cae5c 100644
--- a/tests/data/region_processing/external_repo_test_missing_region/nomenclature.yaml
+++ b/tests/data/region_processing/external_repo_test_missing_region/nomenclature.yaml
@@ -4,7 +4,7 @@ dimensions:
 repositories:
   common-definitions:
     url: https://github.com/IAMconsortium/common-definitions.git/
-    hash: cb85704
+    hash: 091c0fe
 definitions:
   region:
     repository:
diff --git a/tests/data/region_processing/region_aggregation/illegal_mapping_constituent_native_missing.yaml b/tests/data/region_processing/region_aggregation/illegal_mapping_constituent_native_missing.yaml
new file mode 100644
index 0000000..838a532
--- /dev/null
+++ b/tests/data/region_processing/region_aggregation/illegal_mapping_constituent_native_missing.yaml
@@ -0,0 +1,10 @@
+# Constituent region is not defined in native region
+model: model_a
+native_regions:
+  - region_a: alternative_name_a
+  - region_b: alternative_name_b
+common_regions:
+  - common_region_1:
+    - region_a
+    - region_b
+    - region_c
diff --git a/tests/data/validation/validate_data/validate_warning_joined.yaml b/tests/data/validation/validate_data/validate_warning_joined.yaml
new file mode 100644
index 0000000..5ff92b3
--- /dev/null
+++ b/tests/data/validation/validate_data/validate_warning_joined.yaml
@@ -0,0 +1,12 @@
+ - variable: Primary Energy
+   year: 2010
+   validation:
+    - upper_bound: 5
+      lower_bound: 1
+    - warning_level: low
+      upper_bound: 2.5
+      lower_bound: 1
+ - variable: Primary Energy|Coal
+   year: 2010
+   upper_bound: 5
+   lower_bound: 1
diff --git a/tests/data/validation/validate_data/validate_warning_joined_asc.yaml b/tests/data/validation/validate_data/validate_warning_joined_asc.yaml
new file mode 100644
index 0000000..21e484a
--- /dev/null
+++ b/tests/data/validation/validate_data/validate_warning_joined_asc.yaml
@@ -0,0 +1,12 @@
+ - variable: Primary Energy
+   year: 2010
+   validation:
+    - warning_level: low
+      upper_bound: 2.5
+      lower_bound: 1
+    - upper_bound: 5
+      lower_bound: 1
+ - variable: Primary Energy|Coal
+   year: 2010
+   upper_bound: 5
+   lower_bound: 1
diff --git a/tests/data/validation/validate_data/validate_warning.yaml b/tests/data/validation/validate_data/validate_warning_legacy.yaml
similarity index 100%
rename from tests/data/validation/validate_data/validate_warning.yaml
rename to tests/data/validation/validate_data/validate_warning_legacy.yaml
index 3482305..4a4692c 100644
--- a/tests/data/validation/validate_data/validate_warning.yaml
+++ b/tests/data/validation/validate_data/validate_warning_legacy.yaml
@@ -1,12 +1,12 @@
  - variable: Primary Energy
    year: 2010
-   upper_bound: 2.5
+   upper_bound: 5
    lower_bound: 1
-   warning_level: low
  - variable: Primary Energy
    year: 2010
-   upper_bound: 5
+   upper_bound: 2.5
    lower_bound: 1
+   warning_level: low
  - variable: Primary Energy|Coal
    year: 2010
    upper_bound: 5
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 47dafc2..7bbc138 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -291,7 +291,7 @@ def test_cli_validate_data_fails():
     )
 
     assert cli_result.exit_code == 1
-    assert "Collected 5 errors" in str(cli_result.exception)
+    assert "Collected 6 errors" in str(cli_result.exception)
     assert "Asia" in str(cli_result.exception)
     assert "Final Energy|Industry" in str(cli_result.exception)
 
diff --git a/tests/test_codelist.py b/tests/test_codelist.py
index 552161f..f9106b8 100644
--- a/tests/test_codelist.py
+++ b/tests/test_codelist.py
@@ -271,9 +271,18 @@ def test_to_csv():
 @pytest.mark.parametrize(
     "subfolder, match",
     [
-        ("tag_in_str", r"Unexpected character in variable: 'Primary Energy\|{Feul}'"),
-        ("tag_in_list", r"Unexpected character in variable: 'Share\|Coal'"),
-        ("tag_in_dict", r"Unexpected character in variable: 'Primary Energy'"),
+        (
+            "tag_in_str",
+            r"Illegal character\(s\) '{', '}' in name of variable 'Primary Energy\|{Feul}'",
+        ),
+        (
+            "tag_in_list",
+            r"Illegal character\(s\) '{' in info of variable 'Share\|Coal'",
+        ),
+        (
+            "tag_in_dict",
+            r"Illegal character\(s\) '}' in invalid of variable 'Primary Energy'",
+        ),
     ],
 )
 def test_stray_tag_fails(subfolder, match):
@@ -287,7 +296,7 @@ def test_stray_tag_fails(subfolder, match):
 
 def test_illegal_char_fails():
     """Check that illegal character raises expected error."""
-    match = r"Unexpected character in variable: 'Primary Energy\|Coal'"
+    match = r"Illegal character\(s\) '\"' in info of variable 'Primary Energy\|Coal'"
     with raises(ValueError, match=match):
         DataStructureDefinition(
             MODULE_TEST_DATA_DIR / "illegal_chars" / "char_in_str" / "definitions"
diff --git a/tests/test_config.py b/tests/test_config.py
index 73f9efa..a559650 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -93,7 +93,6 @@ def test_config_with_filter(config_file):
 
 
 def test_config_external_repo_mapping_filter():
-
     config = NomenclatureConfig.from_file(
         TEST_DATA_DIR / "config" / "filter_mappings.yaml"
     )
diff --git a/tests/test_region_aggregation.py b/tests/test_region_aggregation.py
index 85b1655..7472084 100644
--- a/tests/test_region_aggregation.py
+++ b/tests/test_region_aggregation.py
@@ -79,6 +79,10 @@ def test_mapping():
             "illegal_mapping_model_only.yaml",
             "one of 'native_regions' and 'common_regions'",
         ),
+        (
+            "illegal_mapping_constituent_native_missing.yaml",
+            "Constituent region\(s\)\n.*\n",
+        ),
     ],
 )
 def test_illegal_mappings(file, error_msg_pattern):
@@ -196,7 +200,7 @@ def test_region_processor_wrong_args():
 
 def test_region_processor_multiple_wrong_mappings(simple_definition):
     # Read in the entire region_aggregation directory and return **all** errors
-    msg = "Collected 9 errors"
+    msg = "Collected 10 errors"
 
     with pytest.raises(ValueError, match=msg):
         RegionProcessor.from_directory(
diff --git a/tests/test_validate_data.py b/tests/test_validate_data.py
index 8d0dbf0..01a4168 100644
--- a/tests/test_validate_data.py
+++ b/tests/test_validate_data.py
@@ -16,8 +16,14 @@ def test_DataValidator_from_file():
                 {
                     "variable": "Final Energy",
                     "year": [2010],
-                    "upper_bound": 2.5,
-                    "lower_bound": 1.0,  # test that integer in yaml is cast to float
+                    "validation": [
+                        {
+                            "variable": "Final Energy",
+                            "year": [2010],
+                            "upper_bound": 2.5,
+                            "lower_bound": 1.0,  # test that integer in yaml is cast to float
+                        }
+                    ],
                 }
             ],
             "file": DATA_VALIDATION_TEST_DIR / "simple_validation.yaml",
@@ -129,26 +135,55 @@ def test_DataValidator_apply_fails(simple_df, file, item_1, item_2, item_3, capl
     assert failed_validation_message in caplog.text
 
 
-def test_DataValidator_validate_with_warning(simple_df, caplog):
+@pytest.mark.parametrize(
+    "file, value",
+    [("joined", 6.0), ("joined", 3.0), ("legacy", 6.0)],
+)
+def test_DataValidator_validate_with_warning(file, value, simple_df, caplog):
+    """Checks that failed validation rows are printed in log."""
+    simple_df._data.iloc[1] = value
     data_validator = DataValidator.from_file(
-        DATA_VALIDATION_TEST_DIR / "validate_warning.yaml"
+        DATA_VALIDATION_TEST_DIR / f"validate_warning_{file}.yaml"
     )
-    with pytest.raises(ValueError, match="Data validation failed"):
-        data_validator.apply(simple_df)
 
     failed_validation_message = (
         "Data validation with error(s)/warning(s) "
-        f"""(file {(DATA_VALIDATION_TEST_DIR / "validate_warning.yaml").relative_to(Path.cwd())}):
+        f"""(file {(DATA_VALIDATION_TEST_DIR / f"validate_warning_{file}.yaml").relative_to(Path.cwd())}):
+  Criteria: variable: ['Primary Energy'], year: [2010], upper_bound: 5.0, lower_bound: 1.0
+         model scenario region        variable   unit  year  value warning_level
+    0  model_a   scen_a  World  Primary Energy  EJ/yr  2010    6.0         error
+    1  model_a   scen_b  World  Primary Energy  EJ/yr  2010    7.0         error"""
+    )
+    if file == "legacy":
+        # prints both error and low warning levels for legacy format
+        # because these are treated as independent validation-criteria
+        failed_validation_message += """
+
   Criteria: variable: ['Primary Energy'], year: [2010], upper_bound: 2.5, lower_bound: 1.0
          model scenario region        variable   unit  year  value warning_level
     0  model_a   scen_a  World  Primary Energy  EJ/yr  2010    6.0           low
-    1  model_a   scen_b  World  Primary Energy  EJ/yr  2010    7.0           low
+    1  model_a   scen_b  World  Primary Energy  EJ/yr  2010    7.0           low"""
 
+    if value == 3.0:
+        # prints each warning level when each is triggered by different rows
+        failed_validation_message = """
   Criteria: variable: ['Primary Energy'], year: [2010], upper_bound: 5.0, lower_bound: 1.0
          model scenario region        variable   unit  year  value warning_level
-    0  model_a   scen_a  World  Primary Energy  EJ/yr  2010    6.0         error
-    1  model_a   scen_b  World  Primary Energy  EJ/yr  2010    7.0         error"""
-    )
+    0  model_a   scen_b  World  Primary Energy  EJ/yr  2010    7.0         error
+
+  Criteria: variable: ['Primary Energy'], year: [2010], upper_bound: 2.5, lower_bound: 1.0
+         model scenario region        variable   unit  year  value warning_level
+    0  model_a   scen_a  World  Primary Energy  EJ/yr  2010    3.0           low"""
 
-    # only prints two of three criteria in df to be validated
+    with pytest.raises(ValueError, match="Data validation failed"):
+        data_validator.apply(simple_df)
     assert failed_validation_message in caplog.text
+
+
+def test_DataValidator_warning_order_fail():
+    """Raises validation error if warnings for same criteria not in descending order."""
+    match = "Validation criteria for .* not in descending order of severity."
+    with pytest.raises(ValueError, match=match):
+        DataValidator.from_file(
+            DATA_VALIDATION_TEST_DIR / "validate_warning_joined_asc.yaml"
+        )