From ac9c80683f1c53581f46416fc5d6b62a834bf3e6 Mon Sep 17 00:00:00 2001
From: Paul Sharp <44529197+DrPaulSharp@users.noreply.github.com>
Date: Tue, 18 Feb 2025 15:40:37 +0000
Subject: [PATCH 1/5] Updates custom error messages

---
 RATapi/controls.py            |  2 +-
 RATapi/project.py             | 10 ++++++----
 RATapi/utils/custom_errors.py |  2 +-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/RATapi/controls.py b/RATapi/controls.py
index 9a36fdee..d696fb57 100644
--- a/RATapi/controls.py
+++ b/RATapi/controls.py
@@ -155,7 +155,7 @@ def warn_setting_incorrect_properties(self, handler: ValidatorFunctionWrapHandle
                 f" controls procedure are:\n    "
                 f"{', '.join(fields.get('procedure', []))}\n",
             }
-            custom_error_list = custom_pydantic_validation_error(exc.errors(), custom_error_msgs)
+            custom_error_list = custom_pydantic_validation_error(exc.errors(include_url=False), custom_error_msgs)
             raise ValidationError.from_exception_data(exc.title, custom_error_list, hide_input=True) from None
 
         if isinstance(model_input, validated_self.__class__):
diff --git a/RATapi/project.py b/RATapi/project.py
index 03f31e5e..dd51b134 100644
--- a/RATapi/project.py
+++ b/RATapi/project.py
@@ -649,13 +649,15 @@ def check_allowed_values(self, attribute: str, field_list: list[str], allowed_va
 
         """
         class_list = getattr(self, attribute)
-        for model in class_list:
+        for index, model in enumerate(class_list):
             for field in field_list:
                 value = getattr(model, field, "")
                 if value and value not in allowed_values:
                     raise ValueError(
-                        f'The value "{value}" in the "{field}" field of "{attribute}" must be defined in '
-                        f'"{values_defined_in[f"{attribute}.{field}"]}".',
+                        f'The value "{value}" used in the "{field}" field at index {index} of "{attribute}" '
+                        f'must be defined in "{values_defined_in[f"{attribute}.{field}"]}". Please either add '
+                        f'"{value}" to "{values_defined_in[f"{attribute}.{field}"]}" before including it in'
+                        f' "{attribute}", or remove it from "{attribute}.{field}" before attempting to delete it.',
                     )
 
     def check_allowed_source(self, attribute: str) -> None:
@@ -945,7 +947,7 @@ def wrapped_func(*args, **kwargs):
                 Project.model_validate(self)
             except ValidationError as exc:
                 class_list.data = previous_state
-                custom_error_list = custom_pydantic_validation_error(exc.errors())
+                custom_error_list = custom_pydantic_validation_error(exc.errors(include_url=False))
                 raise ValidationError.from_exception_data(exc.title, custom_error_list, hide_input=True) from None
             except (TypeError, ValueError):
                 class_list.data = previous_state
diff --git a/RATapi/utils/custom_errors.py b/RATapi/utils/custom_errors.py
index 425cf9ef..5ad0f69a 100644
--- a/RATapi/utils/custom_errors.py
+++ b/RATapi/utils/custom_errors.py
@@ -35,7 +35,7 @@ def custom_pydantic_validation_error(
         if error["type"] in custom_error_msgs:
             custom_error = pydantic_core.PydanticCustomError(error["type"], custom_error_msgs[error["type"]])
         else:
-            custom_error = pydantic_core.PydanticCustomError(error["type"], error["msg"])
+            custom_error = pydantic_core.PydanticCustomError(error["type"], error["msg"].replace(",", ":", 1))
         error["type"] = custom_error
         custom_error_list.append(error)
 

From cb109f82487c3229adfab96d5e7ef22a51fd3cbc Mon Sep 17 00:00:00 2001
From: Paul Sharp <44529197+DrPaulSharp@users.noreply.github.com>
Date: Thu, 17 Apr 2025 15:52:37 +0100
Subject: [PATCH 2/5] Splits error message into two for cross check validation

---
 RATapi/project.py             | 114 +++++++++++++++++++-------
 RATapi/utils/custom_errors.py |   2 +-
 tests/test_project.py         | 150 ++++++++++++++++++++++------------
 3 files changed, 186 insertions(+), 80 deletions(-)

diff --git a/RATapi/project.py b/RATapi/project.py
index dd51b134..a1f98f85 100644
--- a/RATapi/project.py
+++ b/RATapi/project.py
@@ -557,7 +557,6 @@ def update_renamed_models(self) -> "Project":
                         for index, param in all_matches:
                             if param in params:
                                 setattr(project_field[index], param, new_name)
-        self._all_names = self.get_all_names()
         return self
 
     @model_validator(mode="after")
@@ -566,28 +565,45 @@ def cross_check_model_values(self) -> "Project":
         values = ["value_1", "value_2", "value_3", "value_4", "value_5"]
         for field in ["backgrounds", "resolutions"]:
             self.check_allowed_source(field)
-            self.check_allowed_values(field, values, getattr(self, f"{field[:-1]}_parameters").get_names())
+            self.check_allowed_values(
+                field,
+                values,
+                getattr(self, f"{field[:-1]}_parameters").get_names(),
+                self._all_names[f"{field[:-1]}_parameters"],
+            )
 
         self.check_allowed_values(
             "layers",
             ["thickness", "SLD", "SLD_real", "SLD_imaginary", "roughness"],
             self.parameters.get_names(),
+            self._all_names["parameters"],
         )
 
-        self.check_allowed_values("contrasts", ["data"], self.data.get_names())
-        self.check_allowed_values("contrasts", ["background"], self.backgrounds.get_names())
-        self.check_allowed_values("contrasts", ["bulk_in"], self.bulk_in.get_names())
-        self.check_allowed_values("contrasts", ["bulk_out"], self.bulk_out.get_names())
-        self.check_allowed_values("contrasts", ["scalefactor"], self.scalefactors.get_names())
-        self.check_allowed_values("contrasts", ["resolution"], self.resolutions.get_names())
-        self.check_allowed_values("contrasts", ["domain_ratio"], self.domain_ratios.get_names())
+        self.check_allowed_values("contrasts", ["data"], self.data.get_names(), self._all_names["data"])
+        self.check_allowed_values(
+            "contrasts", ["background"], self.backgrounds.get_names(), self._all_names["backgrounds"]
+        )
+        self.check_allowed_values("contrasts", ["bulk_in"], self.bulk_in.get_names(), self._all_names["bulk_in"])
+        self.check_allowed_values("contrasts", ["bulk_out"], self.bulk_out.get_names(), self._all_names["bulk_out"])
+        self.check_allowed_values(
+            "contrasts", ["scalefactor"], self.scalefactors.get_names(), self._all_names["scalefactors"]
+        )
+        self.check_allowed_values(
+            "contrasts", ["resolution"], self.resolutions.get_names(), self._all_names["resolutions"]
+        )
+        self.check_allowed_values(
+            "contrasts", ["domain_ratio"], self.domain_ratios.get_names(), self._all_names["domain_ratios"]
+        )
 
         self.check_contrast_model_allowed_values(
             "contrasts",
             getattr(self, self._contrast_model_field).get_names(),
+            self._all_names[self._contrast_model_field],
             self._contrast_model_field,
         )
-        self.check_contrast_model_allowed_values("domain_contrasts", self.layers.get_names(), "layers")
+        self.check_contrast_model_allowed_values(
+            "domain_contrasts", self.layers.get_names(), self._all_names["layers"], "layers"
+        )
         return self
 
     @model_validator(mode="after")
@@ -606,6 +622,12 @@ def check_protected_parameters(self) -> "Project":
         self._protected_parameters = self.get_all_protected_parameters()
         return self
 
+    @model_validator(mode="after")
+    def update_names(self) -> "Project":
+        """Following validation, update the list of all parameter names."""
+        self._all_names = self.get_all_names()
+        return self
+
     def __str__(self):
         output = ""
         for key, value in self.__dict__.items():
@@ -630,7 +652,9 @@ def get_all_protected_parameters(self):
             for class_list in parameter_class_lists
         }
 
-    def check_allowed_values(self, attribute: str, field_list: list[str], allowed_values: list[str]) -> None:
+    def check_allowed_values(
+        self, attribute: str, field_list: list[str], allowed_values: list[str], previous_values: list[str]
+    ) -> None:
         """Check the values of the given fields in the given model are in the supplied list of allowed values.
 
         Parameters
@@ -641,6 +665,8 @@ def check_allowed_values(self, attribute: str, field_list: list[str], allowed_va
             The fields of the attribute to be checked for valid values.
         allowed_values : list [str]
             The list of allowed values for the fields given in field_list.
+        previous_values : list [str]
+            The list of allowed values for the fields given in field_list after the previous validation.
 
         Raises
         ------
@@ -653,12 +679,19 @@ def check_allowed_values(self, attribute: str, field_list: list[str], allowed_va
             for field in field_list:
                 value = getattr(model, field, "")
                 if value and value not in allowed_values:
-                    raise ValueError(
-                        f'The value "{value}" used in the "{field}" field at index {index} of "{attribute}" '
-                        f'must be defined in "{values_defined_in[f"{attribute}.{field}"]}". Please either add '
-                        f'"{value}" to "{values_defined_in[f"{attribute}.{field}"]}" before including it in'
-                        f' "{attribute}", or remove it from "{attribute}.{field}" before attempting to delete it.',
-                    )
+                    if value in previous_values:
+                        raise ValueError(
+                            f'The value "{value}" used in the "{field}" field at index {index} of "{attribute}" '
+                            f'must be defined in "{values_defined_in[f"{attribute}.{field}"]}". Please remove '
+                            f'"{value}" from "{attribute}{index}.{field}" before attempting to delete it.',
+                        )
+                    else:
+                        raise ValueError(
+                            f'The value "{value}" used in the "{field}" field at index {index} of "{attribute}" '
+                            f'must be defined in "{values_defined_in[f"{attribute}.{field}"]}". Please add '
+                            f'"{value}" to "{values_defined_in[f"{attribute}.{field}"]}" before including it in '
+                            f'"{attribute}".',
+                        )
 
     def check_allowed_source(self, attribute: str) -> None:
         """Check that the source of a background or resolution is defined in the relevant field for its type.
@@ -681,24 +714,37 @@ def check_allowed_source(self, attribute: str) -> None:
 
         """
         class_list = getattr(self, attribute)
-        for model in class_list:
+        for index, model in enumerate(class_list):
             if model.type == TypeOptions.Constant:
                 allowed_values = getattr(self, f"{attribute[:-1]}_parameters").get_names()
+                previous_values = self._all_names[f"{attribute[:-1]}_parameters"]
             elif model.type == TypeOptions.Data:
                 allowed_values = self.data.get_names()
+                previous_values = self._all_names["data"]
             else:
                 allowed_values = self.custom_files.get_names()
+                previous_values = self._all_names["custom_files"]
 
             if (value := model.source) != "" and value not in allowed_values:
-                raise ValueError(
-                    f'The value "{value}" in the "source" field of "{attribute}" must be defined in '
-                    f'"{values_defined_in[f"{attribute}.{model.type}.source"]}".',
-                )
+                if value in previous_values:
+                    raise ValueError(
+                        f'The value "{value}" used in the "source" field at index {index} of "{attribute}" '
+                        f'must be defined in "{values_defined_in[f"{attribute}.{model.type}.source"]}". Please remove '
+                        f'"{value}" from "{attribute}{index}.source" before attempting to delete it.',
+                    )
+                else:
+                    raise ValueError(
+                        f'The value "{value}" used in the "source" field at index {index} of "{attribute}" '
+                        f'must be defined in "{values_defined_in[f"{attribute}.{model.type}.source"]}". Please add '
+                        f'"{value}" to "{values_defined_in[f"{attribute}.{model.type}.source"]}" before including it '
+                        f'in "{attribute}".',
+                    )
 
     def check_contrast_model_allowed_values(
         self,
         contrast_attribute: str,
         allowed_values: list[str],
+        previous_values: list[str],
         allowed_field: str,
     ) -> None:
         """Ensure the contents of the ``model`` for a contrast or domain contrast exist in the required project fields.
@@ -709,6 +755,8 @@ def check_contrast_model_allowed_values(
             The specific contrast attribute of Project being validated (either "contrasts" or "domain_contrasts").
         allowed_values : list [str]
             The list of allowed values for the model of the contrast_attribute.
+        previous_values : list [str]
+            The list of allowed values for the model of the contrast_attribute after the previous validation.
         allowed_field : str
             The name of the field in the project in which the allowed_values are defined.
 
@@ -719,13 +767,21 @@ def check_contrast_model_allowed_values(
 
         """
         class_list = getattr(self, contrast_attribute)
-        for contrast in class_list:
-            model_values = contrast.model
-            if model_values and not all(value in allowed_values for value in model_values):
-                raise ValueError(
-                    f'The values: "{", ".join(str(i) for i in model_values)}" in the "model" field of '
-                    f'"{contrast_attribute}" must be defined in "{allowed_field}".',
-                )
+        for index, contrast in enumerate(class_list):
+            if (model_values := contrast.model) and not all(value in allowed_values for value in model_values):
+                if all(value in previous_values for value in model_values):
+                    raise ValueError(
+                        f'The values: "{", ".join(str(i) for i in model_values)}" used in the "model" field at index '
+                        f'{index} of "{contrast_attribute}" must be defined in "{allowed_field}". Please remove '
+                        f'all unnecessary values from "model" before attempting to delete them.',
+                    )
+                else:
+                    raise ValueError(
+                        f'The values: "{", ".join(str(i) for i in model_values)}" used in the "model" field at index '
+                        f'{index} of "{contrast_attribute}" must be defined in "{allowed_field}". Please add '
+                        f'all required values to "{allowed_field}" '
+                        f'before including them in "{contrast_attribute}".',
+                    )
 
     def get_contrast_model_field(self):
         """Get the field used to define the contents of the "model" field in contrasts.
diff --git a/RATapi/utils/custom_errors.py b/RATapi/utils/custom_errors.py
index 5ad0f69a..425cf9ef 100644
--- a/RATapi/utils/custom_errors.py
+++ b/RATapi/utils/custom_errors.py
@@ -35,7 +35,7 @@ def custom_pydantic_validation_error(
         if error["type"] in custom_error_msgs:
             custom_error = pydantic_core.PydanticCustomError(error["type"], custom_error_msgs[error["type"]])
         else:
-            custom_error = pydantic_core.PydanticCustomError(error["type"], error["msg"].replace(",", ":", 1))
+            custom_error = pydantic_core.PydanticCustomError(error["type"], error["msg"])
         error["type"] = custom_error
         custom_error_list.append(error)
 
diff --git a/tests/test_project.py b/tests/test_project.py
index 82fc6388..eb57c78e 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -661,15 +661,17 @@ def test_rename_models(test_project, model: str, fields: list[str]) -> None:
     ],
 )
 def test_allowed_backgrounds(background_type, expected_field) -> None:
-    """If the source field of the Background model are set to values that are not specified in the background
-    parameters, we should raise a ValidationError.
+    """
+    If the source field of the Background model is set to a value that is not specified in the appropriate ClassList,
+    we should raise a ValidationError.
     """
     test_background = RATapi.models.Background(type=background_type, source="undefined")
     with pytest.raises(
         pydantic.ValidationError,
         match="1 validation error for Project\n  Value error, The value "
-        '"undefined" in the "source" field of "backgrounds" must be '
-        f'defined in "{expected_field}".',
+        '"undefined" used in the "source" field at index 0 of "backgrounds" must be '
+        f'defined in "{expected_field}". Please add "undefined" to "{expected_field}" before including it '
+        f'in "backgrounds".',
     ):
         RATapi.Project(backgrounds=RATapi.ClassList(test_background))
 
@@ -691,8 +693,8 @@ def test_allowed_layers(field: str) -> None:
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" in the "{field}" field of "layers" must be '
-        f'defined in "parameters".',
+        f'"undefined" used in the "{field}" field at index 0 of "layers" must be '
+        f'defined in "parameters". Please add "undefined" to "parameters" before including it in "layers".',
     ):
         RATapi.Project(
             absorption=False,
@@ -725,8 +727,10 @@ def test_allowed_absorption_layers(field: str) -> None:
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" in the "{field}" field of "layers" must be '
-        f'defined in "parameters".',
+        f'"undefined" used in the "{field}" field at index 0 of "layers" must be '
+        f'defined in "parameters". Please add '
+        f'"undefined" to "parameters" before including it '
+        f'in "layers".',
     ):
         RATapi.Project(
             absorption=True,
@@ -757,8 +761,10 @@ def test_allowed_resolutions(resolution_type, expected_field) -> None:
     with pytest.raises(
         pydantic.ValidationError,
         match="1 validation error for Project\n  Value error, The value "
-        '"undefined" in the "source" field of "resolutions" must be '
-        f'defined in "{expected_field}".',
+        '"undefined" used in the "source" field at index 0 of "resolutions" must be '
+        f'defined in "{expected_field}". Please add '
+        f'"undefined" to "{expected_field}" before including it '
+        f'in "resolutions".',
     ):
         RATapi.Project(resolutions=RATapi.ClassList(test_resolution))
 
@@ -782,8 +788,10 @@ def test_allowed_contrasts(field: str, model_name: str) -> None:
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" in the "{field}" field of "contrasts" must be '
-        f'defined in "{model_name}".',
+        f'"undefined" used in the "{field}" field at index 0 of "contrasts" must be '
+        f'defined in "{model_name}". Please add '
+        f'"undefined" to "{model_name}" before including it '
+        f'in "contrasts".',
     ):
         RATapi.Project(calculation=Calculations.Normal, contrasts=RATapi.ClassList(test_contrast))
 
@@ -808,8 +816,10 @@ def test_allowed_contrasts_with_ratio(field: str, model_name: str) -> None:
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" in the "{field}" field of "contrasts" must be '
-        f'defined in "{model_name}".',
+        f'"undefined" used in the "{field}" field at index 0 of "contrasts" must be '
+        f'defined in "{model_name}". Please add '
+        f'"undefined" to "{model_name}" before including it '
+        f'in "contrasts".',
     ):
         RATapi.Project(calculation=Calculations.Domains, contrasts=RATapi.ClassList(test_contrast))
 
@@ -867,8 +877,10 @@ def test_allowed_contrast_models(
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The values: "
-        f'"{", ".join(test_contrast.model)}" in the "model" field of '
-        f'"contrasts" must be defined in "{field_name}".',
+        f'"{", ".join(test_contrast.model)}" used in the "model" field at index 0 of '
+        f'"contrasts" must be defined in "{field_name}". Please add '
+        f'all required values to "{field_name}" '
+        f'before including them in "contrasts".',
     ):
         RATapi.Project(calculation=input_calc, model=input_model, contrasts=RATapi.ClassList(test_contrast))
 
@@ -881,8 +893,10 @@ def test_allowed_domain_contrast_models() -> None:
     with pytest.raises(
         pydantic.ValidationError,
         match="1 validation error for Project\n  Value error, The values: "
-        '"undefined" in the "model" field of "domain_contrasts" must be '
-        'defined in "layers".',
+        '"undefined" used in the "model" field at index 0 of "domain_contrasts" must be '
+        'defined in "layers". Please add '
+        'all required values to "layers" '
+        'before including them in "domain_contrasts".',
     ):
         RATapi.Project(calculation=Calculations.Domains, domain_contrasts=RATapi.ClassList(test_contrast))
 
@@ -934,10 +948,11 @@ def test_get_all_protected_parameters(test_project) -> None:
 )
 def test_check_allowed_values(test_value: str) -> None:
     """We should not raise an error if string values are defined and on the list of allowed values."""
+    allowed_values = ["Substrate Roughness"]
     project = RATapi.Project.model_construct(
         layers=RATapi.ClassList(RATapi.models.Layer(**dict(layer_params, roughness=test_value)))
     )
-    assert project.check_allowed_values("layers", ["roughness"], ["Substrate Roughness"]) is None
+    assert project.check_allowed_values("layers", ["roughness"], allowed_values, allowed_values) is None
 
 
 @pytest.mark.parametrize(
@@ -951,11 +966,13 @@ def test_check_allowed_values_not_on_list(test_value: str) -> None:
     project = RATapi.Project.model_construct(
         layers=RATapi.ClassList(RATapi.models.Layer(**dict(layer_params, roughness=test_value)))
     )
+    allowed_values = ["Substrate Roughness"]
     with pytest.raises(
         ValueError,
-        match=f'The value "{test_value}" in the "roughness" field of "layers" must be defined in "parameters".',
+        match=f'The value "{test_value}" used in the "roughness" field at index 0 of "layers" must be defined in '
+        f'"parameters".',
     ):
-        project.check_allowed_values("layers", ["roughness"], ["Substrate Roughness"])
+        project.check_allowed_values("layers", ["roughness"], allowed_values, allowed_values)
 
 
 @pytest.mark.parametrize(
@@ -1002,7 +1019,7 @@ def test_check_allowed_background_resolution_values_not_on_constant_list(test_va
     )
     with pytest.raises(
         ValueError,
-        match=f'The value "{test_value}" in the "source" field of "backgrounds" must be '
+        match=f'The value "{test_value}" used in the "source" field at index 0 of "backgrounds" must be '
         f'defined in "background_parameters".',
     ):
         project.check_allowed_source(
@@ -1026,7 +1043,8 @@ def test_check_allowed_background_resolution_values_on_data_list(test_value: str
     )
     with pytest.raises(
         ValueError,
-        match=f'The value "{test_value}" in the "source" field of "backgrounds" must be defined in "data".',
+        match=f'The value "{test_value}" used in the "source" field at index 0 of "backgrounds" must be defined in '
+        f'"data".',
     ):
         project.check_allowed_source("backgrounds")
 
@@ -1045,7 +1063,7 @@ def test_check_contrast_model_allowed_values(test_values: list[str]) -> None:
     project = RATapi.Project.model_construct(
         contrasts=RATapi.ClassList(RATapi.models.Contrast(name="Test Contrast", model=test_values)),
     )
-    assert project.check_contrast_model_allowed_values("contrasts", ["Test Layer"], "layers") is None
+    assert project.check_contrast_model_allowed_values("contrasts", ["Test Layer"], ["Test Layer"], "layers") is None
 
 
 @pytest.mark.parametrize(
@@ -1064,10 +1082,10 @@ def test_check_allowed_contrast_model_not_on_list(test_values: list[str]) -> Non
     )
     with pytest.raises(
         ValueError,
-        match=f'The values: "{", ".join(str(i) for i in test_values)}" in the "model" field '
+        match=f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field at index 0 '
         f'of "contrasts" must be defined in "layers".',
     ):
-        project.check_contrast_model_allowed_values("contrasts", ["Test Layer"], "layers")
+        project.check_contrast_model_allowed_values("contrasts", ["Test Layer"], ["Test Layer"], "layers")
 
 
 @pytest.mark.parametrize(
@@ -1155,15 +1173,18 @@ def test_wrap_set(test_project, class_list: str, model_type: str, field: str) ->
     test_attribute = getattr(test_project, class_list)
     orig_class_list = copy.deepcopy(test_attribute)
     class_list_str = f"{class_list}{f'.{model_type}' if model_type else ''}.{field}"
+    index = 0
 
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" in the "{field}" field of "{class_list}" must be '
+        f'"undefined" used in the "{field}" field at index {index} of "{class_list}" must be '
         f"defined in "
-        f'"{RATapi.project.values_defined_in[class_list_str]}".',
+        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add '
+        f'"undefined" to "{RATapi.project.values_defined_in[class_list_str]}" before including it in '
+        f'"{class_list}".',
     ):
-        test_attribute.set_fields(0, **{field: "undefined"})
+        test_attribute.set_fields(index, **{field: "undefined"})
 
     # Ensure invalid model was not changed
     assert test_attribute == orig_class_list
@@ -1189,12 +1210,17 @@ def test_wrap_del(test_project, class_list: str, parameter: str, field: str) ->
     orig_class_list = copy.deepcopy(test_attribute)
     index = test_attribute.index(parameter)
 
+    sub_attribute_name = RATapi.project.model_names_used_in[class_list][0].attribute
+    sub_attribute = getattr(test_project, sub_attribute_name)
+    sub_index = [i for i, _ in enumerate(sub_attribute) if getattr(sub_attribute[i], field) == parameter][0]
+
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"{parameter}" in the "{field}" field of '
-        f'"{RATapi.project.model_names_used_in[class_list][0].attribute}" '
-        f'must be defined in "{class_list}".',
+        f'"{parameter}" used in the "{field}" field at index {sub_index} of '
+        f'"{sub_attribute_name}" '
+        f'must be defined in "{class_list}". Please remove '
+        f'"{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
     ):
         del test_attribute[index]
 
@@ -1229,9 +1255,11 @@ def test_wrap_iadd(test_project, class_list: str, model_type: str, field: str, m
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" in the "{field}" field of "{class_list}" must be '
+        f'"undefined" used in the "{field}" field at index {len(test_attribute)} of "{class_list}" must be '
         f"defined in "
-        f'"{RATapi.project.values_defined_in[class_list_str]}".',
+        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add '
+        f'"undefined" to "{RATapi.project.values_defined_in[class_list_str]}" before including it in '
+        f'"{class_list}".',
     ):
         test_attribute += [input_model(**{**model_params, field: "undefined"})]
 
@@ -1267,9 +1295,11 @@ def test_wrap_append(test_project, class_list: str, model_type: str, field: str,
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" in the "{field}" field of "{class_list}" must be '
+        f'"undefined" used in the "{field}" field at index {len(test_attribute)} of "{class_list}" must be '
         f"defined in "
-        f'"{RATapi.project.values_defined_in[class_list_str]}".',
+        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add '
+        f'"undefined" to "{RATapi.project.values_defined_in[class_list_str]}" before including it in '
+        f'"{class_list}".',
     ):
         test_attribute.append(input_model(**{**model_params, field: "undefined"}))
 
@@ -1300,15 +1330,18 @@ def test_wrap_insert(test_project, class_list: str, model_type: str, field: str,
     orig_class_list = copy.deepcopy(test_attribute)
     input_model = model_classes[class_list]
     class_list_str = f"{class_list}{f'.{model_type}' if model_type else ''}.{field}"
+    index = 0
 
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" in the "{field}" field of "{class_list}" must be '
+        f'"undefined" used in the "{field}" field at index {index} of "{class_list}" must be '
         f"defined in "
-        f'"{RATapi.project.values_defined_in[class_list_str]}".',
+        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add '
+        f'"undefined" to "{RATapi.project.values_defined_in[class_list_str]}" before including it in '
+        f'"{class_list}".',
     ):
-        test_attribute.insert(0, input_model(**{**model_params, field: "undefined"}))
+        test_attribute.insert(index, input_model(**{**model_params, field: "undefined"}))
 
     # Ensure invalid model was not inserted
     assert test_attribute == orig_class_list
@@ -1371,12 +1404,17 @@ def test_wrap_pop(test_project, class_list: str, parameter: str, field: str) ->
     orig_class_list = copy.deepcopy(test_attribute)
     index = test_attribute.index(parameter)
 
+    sub_attribute_name = RATapi.project.model_names_used_in[class_list][0].attribute
+    sub_attribute = getattr(test_project, sub_attribute_name)
+    sub_index = [i for i, _ in enumerate(sub_attribute) if getattr(sub_attribute[i], field) == parameter][0]
+
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"{parameter}" in the "{field}" field of '
-        f'"{RATapi.project.model_names_used_in[class_list][0].attribute}" '
-        f'must be defined in "{class_list}".',
+        f'"{parameter}" used in the "{field}" field at index {sub_index} of '
+        f'"{sub_attribute_name}" '
+        f'must be defined in "{class_list}". Please remove '
+        f'"{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
     ):
         test_attribute.pop(index)
 
@@ -1403,12 +1441,17 @@ def test_wrap_remove(test_project, class_list: str, parameter: str, field: str)
     test_attribute = getattr(test_project, class_list)
     orig_class_list = copy.deepcopy(test_attribute)
 
+    sub_attribute_name = RATapi.project.model_names_used_in[class_list][0].attribute
+    sub_attribute = getattr(test_project, sub_attribute_name)
+    sub_index = [i for i, _ in enumerate(sub_attribute) if getattr(sub_attribute[i], field) == parameter][0]
+
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"{parameter}" in the "{field}" field of '
-        f'"{RATapi.project.model_names_used_in[class_list][0].attribute}" '
-        f'must be defined in "{class_list}".',
+        f'"{parameter}" used in the "{field}" field at index {sub_index} of '
+        f'"{sub_attribute_name}" '
+        f'must be defined in "{class_list}". Please remove '
+        f'"{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
     ):
         test_attribute.remove(parameter)
 
@@ -1435,12 +1478,17 @@ def test_wrap_clear(test_project, class_list: str, parameter: str, field: str) -
     test_attribute = getattr(test_project, class_list)
     orig_class_list = copy.deepcopy(test_attribute)
 
+    sub_attribute_name = RATapi.project.model_names_used_in[class_list][0].attribute
+    sub_attribute = getattr(test_project, sub_attribute_name)
+    sub_index = [i for i, _ in enumerate(sub_attribute) if getattr(sub_attribute[i], field) == parameter][0]
+
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"{parameter}" in the "{field}" field of '
-        f'"{RATapi.project.model_names_used_in[class_list][0].attribute}" '
-        f'must be defined in "{class_list}".',
+        f'"{parameter}" used in the "{field}" field at index {sub_index} of '
+        f'"{sub_attribute_name}" '
+        f'must be defined in "{class_list}". Please remove '
+        f'"{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
     ):
         test_attribute.clear()
 
@@ -1475,9 +1523,11 @@ def test_wrap_extend(test_project, class_list: str, model_type: str, field: str,
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" in the "{field}" field of "{class_list}" must be '
+        f'"undefined" used in the "{field}" field at index {len(test_attribute)} of "{class_list}" must be '
         f"defined in "
-        f'"{RATapi.project.values_defined_in[class_list_str]}".',
+        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add '
+        f'"undefined" to "{RATapi.project.values_defined_in[class_list_str]}" before including it in '
+        f'"{class_list}".',
     ):
         test_attribute.extend([input_model(**{**model_params, field: "undefined"})])
 

From f0dc83fe9c509762febb862f53722d3523bab8f6 Mon Sep 17 00:00:00 2001
From: Paul Sharp <44529197+DrPaulSharp@users.noreply.github.com>
Date: Tue, 22 Apr 2025 13:42:44 +0100
Subject: [PATCH 3/5] Tidies up tests and improves coverage

---
 RATapi/project.py     |   5 +-
 tests/test_project.py | 219 ++++++++++++++++++++----------------------
 2 files changed, 107 insertions(+), 117 deletions(-)

diff --git a/RATapi/project.py b/RATapi/project.py
index a1f98f85..5f83c963 100644
--- a/RATapi/project.py
+++ b/RATapi/project.py
@@ -1038,9 +1038,8 @@ def try_relative_to(path: Path, relative_to: Path) -> str:
     else:
         warnings.warn(
             "Could not save custom file path as relative to the project directory, "
-            "which means that it may not work on other devices."
-            "If you would like to share your project, make sure your custom files "
-            "are in a subfolder of the project save location.",
+            "which means that it may not work on other devices. If you would like to share your project, "
+            "make sure your custom files are in a subfolder of the project save location.",
             stacklevel=2,
         )
         return str(path.resolve())
diff --git a/tests/test_project.py b/tests/test_project.py
index eb57c78e..c6bbc5c5 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -140,7 +140,7 @@ def test_classlists(test_project) -> None:
     """The ClassLists in the "Project" model should contain instances of the models given by the dictionary
     "model_in_classlist".
     """
-    for model in (fields := RATapi.Project.model_fields):
+    for model in (fields := test_project.model_fields):
         if get_origin(fields[model].annotation) == RATapi.ClassList:
             class_list = getattr(test_project, model)
             assert class_list._class_handle == get_args(fields[model].annotation)[0]
@@ -173,11 +173,9 @@ def test_initialise_wrong_classes(input_model: Callable, model_params: dict) ->
     """If the "Project" model is initialised with incorrect classes, we should raise a ValidationError."""
     with pytest.raises(
         pydantic.ValidationError,
-        match=(
-            "1 validation error for Project\nparameters\n"
-            "  Value error, This ClassList only supports elements of type Parameter. In the input list:\n"
-            f"    index 0 is of type {input_model.__name__}"
-        ),
+        match="1 validation error for Project\nparameters\n  "
+        "Value error, This ClassList only supports elements of type Parameter. In the input list:\n"
+        f"    index 0 is of type {input_model.__name__}",
     ):
         RATapi.Project(parameters=RATapi.ClassList(input_model(**model_params)))
 
@@ -201,8 +199,8 @@ def test_initialise_wrong_layers(
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\nlayers\n  Value error, "
-        f'"The layers attribute contains {input_model.__name__}s, but the absorption parameter is {absorption}. '
-        f'The attribute should be a ClassList of {actual_model_name} instead."',
+        f'"The layers attribute contains {input_model.__name__}s, but the absorption parameter is '
+        f'{absorption}. The attribute should be a ClassList of {actual_model_name} instead."',
     ):
         RATapi.Project(absorption=absorption, layers=RATapi.ClassList(input_model(**model_params)))
 
@@ -236,8 +234,8 @@ def test_initialise_ambiguous_layers(absorption: bool, model: RATapi.models.RATM
 def test_initialise_wrong_contrasts(
     input_model: RATapi.models.RATModel, calculation: Calculations, actual_model_name: str
 ) -> None:
-    """If the "Project" model is initialised with the incorrect contrast model given the value of calculation, we should
-    raise a ValidationError.
+    """If the "Project" model is initialised with the incorrect contrast model given the value of calculation, we
+    should raise a ValidationError.
     """
     word = "without" if calculation == Calculations.Domains else "with"
     with pytest.raises(
@@ -353,8 +351,8 @@ def test_assign_wrong_layers(
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\nlayers\n  Value error, "
-        f'"The layers attribute contains {wrong_input_model.__name__}s, but the absorption parameter is {absorption}. '
-        f'The attribute should be a ClassList of {actual_model_name} instead."',
+        f'"The layers attribute contains {wrong_input_model.__name__}s, but the absorption parameter is '
+        f'{absorption}. The attribute should be a ClassList of {actual_model_name} instead."',
     ):
         project.layers = RATapi.ClassList(wrong_input_model(**model_params))
 
@@ -373,8 +371,8 @@ def test_assign_wrong_contrasts(wrong_input_model: Callable, calculation: Calcul
     with pytest.raises(
         pydantic.ValidationError,
         match=f"1 validation error for Project\ncontrasts\n"
-        f'  Value error, "The contrasts attribute contains contrasts {word} ratio, '
-        f'but the calculation is {calculation}"',
+        f'  Value error, "The contrasts attribute contains contrasts {word} ratio, but the calculation is '
+        f'{calculation}"',
     ):
         project.contrasts = RATapi.ClassList(wrong_input_model())
 
@@ -608,8 +606,8 @@ def test_check_protected_parameters(delete_operation) -> None:
 
     with pytest.raises(
         pydantic.ValidationError,
-        match="1 validation error for Project\n  Value error, Can't delete"
-        " the protected parameters: Substrate Roughness",
+        match="1 validation error for Project\n  Value error, "
+        "Can't delete the protected parameters: Substrate Roughness",
     ):
         eval(delete_operation)
 
@@ -668,10 +666,9 @@ def test_allowed_backgrounds(background_type, expected_field) -> None:
     test_background = RATapi.models.Background(type=background_type, source="undefined")
     with pytest.raises(
         pydantic.ValidationError,
-        match="1 validation error for Project\n  Value error, The value "
-        '"undefined" used in the "source" field at index 0 of "backgrounds" must be '
-        f'defined in "{expected_field}". Please add "undefined" to "{expected_field}" before including it '
-        f'in "backgrounds".',
+        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "source" field '
+        f'at index 0 of "backgrounds" must be defined in "{expected_field}". Please add "undefined" to '
+        f'"{expected_field}" before including it in "backgrounds".',
     ):
         RATapi.Project(backgrounds=RATapi.ClassList(test_background))
 
@@ -692,9 +689,9 @@ def test_allowed_layers(field: str) -> None:
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" used in the "{field}" field at index 0 of "layers" must be '
-        f'defined in "parameters". Please add "undefined" to "parameters" before including it in "layers".',
+        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
+        f'at index 0 of "layers" must be defined in "parameters". Please add "undefined" to "parameters" '
+        f'before including it in "layers".',
     ):
         RATapi.Project(
             absorption=False,
@@ -726,11 +723,9 @@ def test_allowed_absorption_layers(field: str) -> None:
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" used in the "{field}" field at index 0 of "layers" must be '
-        f'defined in "parameters". Please add '
-        f'"undefined" to "parameters" before including it '
-        f'in "layers".',
+        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
+        f'at index 0 of "layers" must be defined in "parameters". Please add "undefined" to "parameters" '
+        f'before including it in "layers".',
     ):
         RATapi.Project(
             absorption=True,
@@ -760,11 +755,9 @@ def test_allowed_resolutions(resolution_type, expected_field) -> None:
     test_resolution = RATapi.models.Resolution(type=resolution_type, source="undefined")
     with pytest.raises(
         pydantic.ValidationError,
-        match="1 validation error for Project\n  Value error, The value "
-        '"undefined" used in the "source" field at index 0 of "resolutions" must be '
-        f'defined in "{expected_field}". Please add '
-        f'"undefined" to "{expected_field}" before including it '
-        f'in "resolutions".',
+        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "source" field '
+        f'at index 0 of "resolutions" must be defined in "{expected_field}". Please add "undefined" to '
+        f'"{expected_field}" before including it in "resolutions".',
     ):
         RATapi.Project(resolutions=RATapi.ClassList(test_resolution))
 
@@ -787,11 +780,9 @@ def test_allowed_contrasts(field: str, model_name: str) -> None:
     test_contrast = RATapi.models.Contrast(**{field: "undefined"})
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" used in the "{field}" field at index 0 of "contrasts" must be '
-        f'defined in "{model_name}". Please add '
-        f'"undefined" to "{model_name}" before including it '
-        f'in "contrasts".',
+        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
+        f'at index 0 of "contrasts" must be defined in "{model_name}". Please add "undefined" to '
+        f'"{model_name}" before including it in "contrasts".',
     ):
         RATapi.Project(calculation=Calculations.Normal, contrasts=RATapi.ClassList(test_contrast))
 
@@ -815,11 +806,9 @@ def test_allowed_contrasts_with_ratio(field: str, model_name: str) -> None:
     test_contrast = RATapi.models.ContrastWithRatio(**{field: "undefined"})
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" used in the "{field}" field at index 0 of "contrasts" must be '
-        f'defined in "{model_name}". Please add '
-        f'"undefined" to "{model_name}" before including it '
-        f'in "contrasts".',
+        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
+        f'at index 0 of "contrasts" must be defined in "{model_name}". Please add "undefined" to '
+        f'"{model_name}" before including it in "contrasts".',
     ):
         RATapi.Project(calculation=Calculations.Domains, contrasts=RATapi.ClassList(test_contrast))
 
@@ -876,11 +865,9 @@ def test_allowed_contrast_models(
     """
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The values: "
-        f'"{", ".join(test_contrast.model)}" used in the "model" field at index 0 of '
-        f'"contrasts" must be defined in "{field_name}". Please add '
-        f'all required values to "{field_name}" '
-        f'before including them in "contrasts".',
+        match=f'1 validation error for Project\n  Value error, The values: "{", ".join(test_contrast.model)}" '
+        f'used in the "model" field at index 0 of "contrasts" must be defined in "{field_name}". Please add '
+        f'all required values to "{field_name}" before including them in "contrasts".',
     ):
         RATapi.Project(calculation=input_calc, model=input_model, contrasts=RATapi.ClassList(test_contrast))
 
@@ -892,11 +879,9 @@ def test_allowed_domain_contrast_models() -> None:
     test_contrast = RATapi.models.DomainContrast(name="Test Domain Contrast", model=["undefined"])
     with pytest.raises(
         pydantic.ValidationError,
-        match="1 validation error for Project\n  Value error, The values: "
-        '"undefined" used in the "model" field at index 0 of "domain_contrasts" must be '
-        'defined in "layers". Please add '
-        'all required values to "layers" '
-        'before including them in "domain_contrasts".',
+        match='1 validation error for Project\n  Value error, The values: "undefined" used in the "model" field '
+        'at index 0 of "domain_contrasts" must be defined in "layers". Please add all required values to '
+        '"layers" before including them in "domain_contrasts".',
     ):
         RATapi.Project(calculation=Calculations.Domains, domain_contrasts=RATapi.ClassList(test_contrast))
 
@@ -970,7 +955,7 @@ def test_check_allowed_values_not_on_list(test_value: str) -> None:
     with pytest.raises(
         ValueError,
         match=f'The value "{test_value}" used in the "roughness" field at index 0 of "layers" must be defined in '
-        f'"parameters".',
+        f'"parameters". Please add "{test_value}" to "parameters" before including it in "layers".',
     ):
         project.check_allowed_values("layers", ["roughness"], allowed_values, allowed_values)
 
@@ -1019,8 +1004,9 @@ def test_check_allowed_background_resolution_values_not_on_constant_list(test_va
     )
     with pytest.raises(
         ValueError,
-        match=f'The value "{test_value}" used in the "source" field at index 0 of "backgrounds" must be '
-        f'defined in "background_parameters".',
+        match=f'The value "{test_value}" used in the "source" field at index 0 of "backgrounds" must be defined '
+        f'in "background_parameters". Please add "{test_value}" to "background_parameters" before including '
+        f'it in "backgrounds".',
     ):
         project.check_allowed_source(
             "backgrounds",
@@ -1043,8 +1029,8 @@ def test_check_allowed_background_resolution_values_on_data_list(test_value: str
     )
     with pytest.raises(
         ValueError,
-        match=f'The value "{test_value}" used in the "source" field at index 0 of "backgrounds" must be defined in '
-        f'"data".',
+        match=f'The value "{test_value}" used in the "source" field at index 0 of "backgrounds" must be defined '
+        f'in "data". Please add "{test_value}" to "data" before including it in "backgrounds".',
     ):
         project.check_allowed_source("backgrounds")
 
@@ -1082,12 +1068,36 @@ def test_check_allowed_contrast_model_not_on_list(test_values: list[str]) -> Non
     )
     with pytest.raises(
         ValueError,
-        match=f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field at index 0 '
-        f'of "contrasts" must be defined in "layers".',
+        match=f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field at index 0 of '
+        f'"contrasts" must be defined in "layers". Please add all required values to "layers" before '
+        f'including them in "contrasts".',
     ):
         project.check_contrast_model_allowed_values("contrasts", ["Test Layer"], ["Test Layer"], "layers")
 
 
+@pytest.mark.parametrize(
+    "test_values",
+    [
+        ["Test Layer"],
+        ["Test Layer", "Undefined Param"],
+    ],
+)
+def test_check_allowed_contrast_model_removed_from_list(test_values: list[str]) -> None:
+    """If string values are defined in a non-empty list and any of them have been removed from the list of allowed
+    values we should raise a ValueError.
+    """
+    project = RATapi.Project.model_construct(
+        contrasts=RATapi.ClassList(RATapi.models.Contrast(name="Test Contrast", model=test_values)),
+    )
+    with pytest.raises(
+        ValueError,
+        match=f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field at index 0 of '
+        f'"contrasts" must be defined in "layers". Please remove all unnecessary values from "model" before '
+        f"attempting to delete them.",
+    ):
+        project.check_contrast_model_allowed_values("contrasts", [], ["Test Layer", "Undefined Param"], "layers")
+
+
 @pytest.mark.parametrize(
     ["input_calc", "input_model", "expected_field_name"],
     [
@@ -1177,12 +1187,10 @@ def test_wrap_set(test_project, class_list: str, model_type: str, field: str) ->
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" used in the "{field}" field at index {index} of "{class_list}" must be '
-        f"defined in "
-        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add '
-        f'"undefined" to "{RATapi.project.values_defined_in[class_list_str]}" before including it in '
-        f'"{class_list}".',
+        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
+        f'at index {index} of "{class_list}" must be defined in '
+        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
+        f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".',
     ):
         test_attribute.set_fields(index, **{field: "undefined"})
 
@@ -1216,11 +1224,9 @@ def test_wrap_del(test_project, class_list: str, parameter: str, field: str) ->
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"{parameter}" used in the "{field}" field at index {sub_index} of '
-        f'"{sub_attribute_name}" '
-        f'must be defined in "{class_list}". Please remove '
-        f'"{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
+        match=f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" '
+        f'field at index {sub_index} of "{sub_attribute_name}" must be defined in "{class_list}". '
+        f'Please remove "{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
     ):
         del test_attribute[index]
 
@@ -1254,12 +1260,10 @@ def test_wrap_iadd(test_project, class_list: str, model_type: str, field: str, m
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" used in the "{field}" field at index {len(test_attribute)} of "{class_list}" must be '
-        f"defined in "
-        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add '
-        f'"undefined" to "{RATapi.project.values_defined_in[class_list_str]}" before including it in '
-        f'"{class_list}".',
+        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
+        f'at index {len(test_attribute)} of "{class_list}" must be defined in '
+        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
+        f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".',
     ):
         test_attribute += [input_model(**{**model_params, field: "undefined"})]
 
@@ -1294,12 +1298,10 @@ def test_wrap_append(test_project, class_list: str, model_type: str, field: str,
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" used in the "{field}" field at index {len(test_attribute)} of "{class_list}" must be '
-        f"defined in "
-        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add '
-        f'"undefined" to "{RATapi.project.values_defined_in[class_list_str]}" before including it in '
-        f'"{class_list}".',
+        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}(" field '
+        f'at index {len(test_attribute)} of "){class_list}" must be defined in '
+        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
+        f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".',
     ):
         test_attribute.append(input_model(**{**model_params, field: "undefined"}))
 
@@ -1334,12 +1336,10 @@ def test_wrap_insert(test_project, class_list: str, model_type: str, field: str,
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" used in the "{field}" field at index {index} of "{class_list}" must be '
-        f"defined in "
-        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add '
-        f'"undefined" to "{RATapi.project.values_defined_in[class_list_str]}" before including it in '
-        f'"{class_list}".',
+        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
+        f'at index {index} of "{class_list}" must be defined in '
+        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
+        f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".',
     ):
         test_attribute.insert(index, input_model(**{**model_params, field: "undefined"}))
 
@@ -1410,11 +1410,9 @@ def test_wrap_pop(test_project, class_list: str, parameter: str, field: str) ->
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"{parameter}" used in the "{field}" field at index {sub_index} of '
-        f'"{sub_attribute_name}" '
-        f'must be defined in "{class_list}". Please remove '
-        f'"{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
+        match=f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" '
+        f'field at index {sub_index} of "{sub_attribute_name}" must be defined in "{class_list}". Please '
+        f'remove "{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
     ):
         test_attribute.pop(index)
 
@@ -1447,11 +1445,9 @@ def test_wrap_remove(test_project, class_list: str, parameter: str, field: str)
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"{parameter}" used in the "{field}" field at index {sub_index} of '
-        f'"{sub_attribute_name}" '
-        f'must be defined in "{class_list}". Please remove '
-        f'"{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
+        match=f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" '
+        f'field at index {sub_index} of "{sub_attribute_name}" must be defined in "{class_list}". Please '
+        f'remove "{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
     ):
         test_attribute.remove(parameter)
 
@@ -1484,11 +1480,9 @@ def test_wrap_clear(test_project, class_list: str, parameter: str, field: str) -
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"{parameter}" used in the "{field}" field at index {sub_index} of '
-        f'"{sub_attribute_name}" '
-        f'must be defined in "{class_list}". Please remove '
-        f'"{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
+        match=f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" '
+        f'field at index {sub_index} of "{sub_attribute_name}" must be defined in "{class_list}". Please '
+        f'remove "{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
     ):
         test_attribute.clear()
 
@@ -1522,12 +1516,10 @@ def test_wrap_extend(test_project, class_list: str, model_type: str, field: str,
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f"1 validation error for Project\n  Value error, The value "
-        f'"undefined" used in the "{field}" field at index {len(test_attribute)} of "{class_list}" must be '
-        f"defined in "
-        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add '
-        f'"undefined" to "{RATapi.project.values_defined_in[class_list_str]}" before including it in '
-        f'"{class_list}".',
+        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
+        f'at index {len(test_attribute)} of "{class_list}" must be defined in '
+        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
+        f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".',
     ):
         test_attribute.extend([input_model(**{**model_params, field: "undefined"})])
 
@@ -1568,7 +1560,7 @@ def test_save_load(project, request):
     for file in original_project.custom_files:
         file.path = file.path.resolve()
 
-    for field in RATapi.Project.model_fields:
+    for field in original_project.model_fields:
         assert getattr(converted_project, field) == getattr(original_project, field)
 
 
@@ -1589,9 +1581,8 @@ def test_relative_paths_warning():
 
     with pytest.warns(
         match="Could not save custom file path as relative to the project directory, "
-        "which means that it may not work on other devices."
-        "If you would like to share your project, make sure your custom files "
-        "are in a subfolder of the project save location.",
+        "which means that it may not work on other devices. If you would like to share your project, "
+        "make sure your custom files are in a subfolder of the project save location.",
     ):
         assert (
             Path(RATapi.project.try_relative_to(data_path, relative_path))

From b054642ed7b0b6643f123b214dea2a55913de186 Mon Sep 17 00:00:00 2001
From: Paul Sharp <44529197+DrPaulSharp@users.noreply.github.com>
Date: Wed, 23 Apr 2025 12:01:30 +0100
Subject: [PATCH 4/5] Reformats error messages

---
 RATapi/project.py     |  38 +++++----
 tests/test_project.py | 182 ++++++++++++++++++++++++++----------------
 2 files changed, 131 insertions(+), 89 deletions(-)

diff --git a/RATapi/project.py b/RATapi/project.py
index 5f83c963..f2e13a6f 100644
--- a/RATapi/project.py
+++ b/RATapi/project.py
@@ -681,16 +681,15 @@ def check_allowed_values(
                 if value and value not in allowed_values:
                     if value in previous_values:
                         raise ValueError(
-                            f'The value "{value}" used in the "{field}" field at index {index} of "{attribute}" '
-                            f'must be defined in "{values_defined_in[f"{attribute}.{field}"]}". Please remove '
-                            f'"{value}" from "{attribute}{index}.{field}" before attempting to delete it.',
+                            f'The value "{value}" used in the "{field}" field of {attribute}[{index}] must be defined '
+                            f'in "{values_defined_in[f"{attribute}.{field}"]}". Please remove "{value}" from '
+                            f'"{attribute}[{index}].{field}" before attempting to delete it.',
                         )
                     else:
                         raise ValueError(
-                            f'The value "{value}" used in the "{field}" field at index {index} of "{attribute}" '
-                            f'must be defined in "{values_defined_in[f"{attribute}.{field}"]}". Please add '
-                            f'"{value}" to "{values_defined_in[f"{attribute}.{field}"]}" before including it in '
-                            f'"{attribute}".',
+                            f'The value "{value}" used in the "{field}" field of {attribute}[{index}] must be defined '
+                            f'in "{values_defined_in[f"{attribute}.{field}"]}". Please add "{value}" to '
+                            f'"{values_defined_in[f"{attribute}.{field}"]}" before including it in "{attribute}".',
                         )
 
     def check_allowed_source(self, attribute: str) -> None:
@@ -728,16 +727,16 @@ def check_allowed_source(self, attribute: str) -> None:
             if (value := model.source) != "" and value not in allowed_values:
                 if value in previous_values:
                     raise ValueError(
-                        f'The value "{value}" used in the "source" field at index {index} of "{attribute}" '
-                        f'must be defined in "{values_defined_in[f"{attribute}.{model.type}.source"]}". Please remove '
-                        f'"{value}" from "{attribute}{index}.source" before attempting to delete it.',
+                        f'The value "{value}" used in the "source" field of {attribute}[{index}] must be defined in '
+                        f'"{values_defined_in[f"{attribute}.{model.type}.source"]}". Please remove "{value}" from '
+                        f'"{attribute}[{index}].source" before attempting to delete it.',
                     )
                 else:
                     raise ValueError(
-                        f'The value "{value}" used in the "source" field at index {index} of "{attribute}" '
-                        f'must be defined in "{values_defined_in[f"{attribute}.{model.type}.source"]}". Please add '
-                        f'"{value}" to "{values_defined_in[f"{attribute}.{model.type}.source"]}" before including it '
-                        f'in "{attribute}".',
+                        f'The value "{value}" used in the "source" field of {attribute}[{index}] must be defined in '
+                        f'"{values_defined_in[f"{attribute}.{model.type}.source"]}". Please add "{value}" to '
+                        f'"{values_defined_in[f"{attribute}.{model.type}.source"]}" before including it in '
+                        f'"{attribute}".',
                     )
 
     def check_contrast_model_allowed_values(
@@ -771,16 +770,15 @@ def check_contrast_model_allowed_values(
             if (model_values := contrast.model) and not all(value in allowed_values for value in model_values):
                 if all(value in previous_values for value in model_values):
                     raise ValueError(
-                        f'The values: "{", ".join(str(i) for i in model_values)}" used in the "model" field at index '
-                        f'{index} of "{contrast_attribute}" must be defined in "{allowed_field}". Please remove '
+                        f'The values: "{", ".join(str(i) for i in model_values)}" used in the "model" field of '
+                        f'{contrast_attribute}[{index}] must be defined in "{allowed_field}". Please remove '
                         f'all unnecessary values from "model" before attempting to delete them.',
                     )
                 else:
                     raise ValueError(
-                        f'The values: "{", ".join(str(i) for i in model_values)}" used in the "model" field at index '
-                        f'{index} of "{contrast_attribute}" must be defined in "{allowed_field}". Please add '
-                        f'all required values to "{allowed_field}" '
-                        f'before including them in "{contrast_attribute}".',
+                        f'The values: "{", ".join(str(i) for i in model_values)}" used in the "model" field of '
+                        f'{contrast_attribute}[{index}] must be defined in "{allowed_field}". Please add '
+                        f'all required values to "{allowed_field}" before including them in "{contrast_attribute}".',
                     )
 
     def get_contrast_model_field(self):
diff --git a/tests/test_project.py b/tests/test_project.py
index c6bbc5c5..82d1a8f7 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -1,6 +1,7 @@
 """Test the project module."""
 
 import copy
+import re
 import tempfile
 import warnings
 from pathlib import Path
@@ -666,9 +667,11 @@ def test_allowed_backgrounds(background_type, expected_field) -> None:
     test_background = RATapi.models.Background(type=background_type, source="undefined")
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "source" field '
-        f'at index 0 of "backgrounds" must be defined in "{expected_field}". Please add "undefined" to '
-        f'"{expected_field}" before including it in "backgrounds".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "undefined" used in the "source" field of '
+            f'backgrounds[0] must be defined in "{expected_field}". Please add "undefined" to "{expected_field}" '
+            f'before including it in "backgrounds".'
+        ),
     ):
         RATapi.Project(backgrounds=RATapi.ClassList(test_background))
 
@@ -689,9 +692,11 @@ def test_allowed_layers(field: str) -> None:
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
-        f'at index 0 of "layers" must be defined in "parameters". Please add "undefined" to "parameters" '
-        f'before including it in "layers".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" '
+            f'field of layers[0] must be defined in "parameters". Please add "undefined" to "parameters" '
+            f'before including it in "layers".'
+        ),
     ):
         RATapi.Project(
             absorption=False,
@@ -723,9 +728,11 @@ def test_allowed_absorption_layers(field: str) -> None:
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
-        f'at index 0 of "layers" must be defined in "parameters". Please add "undefined" to "parameters" '
-        f'before including it in "layers".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field of '
+            f'layers[0] must be defined in "parameters". Please add "undefined" to "parameters" before including it '
+            f'in "layers".'
+        ),
     ):
         RATapi.Project(
             absorption=True,
@@ -755,9 +762,11 @@ def test_allowed_resolutions(resolution_type, expected_field) -> None:
     test_resolution = RATapi.models.Resolution(type=resolution_type, source="undefined")
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "source" field '
-        f'at index 0 of "resolutions" must be defined in "{expected_field}". Please add "undefined" to '
-        f'"{expected_field}" before including it in "resolutions".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "undefined" used in the "source" field of '
+            f'resolutions[0] must be defined in "{expected_field}". Please add "undefined" to "{expected_field}" '
+            f'before including it in "resolutions".'
+        ),
     ):
         RATapi.Project(resolutions=RATapi.ClassList(test_resolution))
 
@@ -780,9 +789,11 @@ def test_allowed_contrasts(field: str, model_name: str) -> None:
     test_contrast = RATapi.models.Contrast(**{field: "undefined"})
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
-        f'at index 0 of "contrasts" must be defined in "{model_name}". Please add "undefined" to '
-        f'"{model_name}" before including it in "contrasts".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field of '
+            f'contrasts[0] must be defined in "{model_name}". Please add "undefined" to "{model_name}" before '
+            f'including it in "contrasts".'
+        ),
     ):
         RATapi.Project(calculation=Calculations.Normal, contrasts=RATapi.ClassList(test_contrast))
 
@@ -806,9 +817,11 @@ def test_allowed_contrasts_with_ratio(field: str, model_name: str) -> None:
     test_contrast = RATapi.models.ContrastWithRatio(**{field: "undefined"})
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
-        f'at index 0 of "contrasts" must be defined in "{model_name}". Please add "undefined" to '
-        f'"{model_name}" before including it in "contrasts".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field of '
+            f'contrasts[0] must be defined in "{model_name}". Please add "undefined" to "{model_name}" before '
+            f'including it in "contrasts".'
+        ),
     ):
         RATapi.Project(calculation=Calculations.Domains, contrasts=RATapi.ClassList(test_contrast))
 
@@ -865,9 +878,11 @@ def test_allowed_contrast_models(
     """
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The values: "{", ".join(test_contrast.model)}" '
-        f'used in the "model" field at index 0 of "contrasts" must be defined in "{field_name}". Please add '
-        f'all required values to "{field_name}" before including them in "contrasts".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The values: "{", ".join(test_contrast.model)}" used in '
+            f'the "model" field of contrasts[0] must be defined in "{field_name}". Please add all required values to '
+            f'"{field_name}" before including them in "contrasts".'
+        ),
     ):
         RATapi.Project(calculation=input_calc, model=input_model, contrasts=RATapi.ClassList(test_contrast))
 
@@ -879,9 +894,11 @@ def test_allowed_domain_contrast_models() -> None:
     test_contrast = RATapi.models.DomainContrast(name="Test Domain Contrast", model=["undefined"])
     with pytest.raises(
         pydantic.ValidationError,
-        match='1 validation error for Project\n  Value error, The values: "undefined" used in the "model" field '
-        'at index 0 of "domain_contrasts" must be defined in "layers". Please add all required values to '
-        '"layers" before including them in "domain_contrasts".',
+        match=re.escape(
+            '1 validation error for Project\n  Value error, The values: "undefined" used in the "model" field of '
+            'domain_contrasts[0] must be defined in "layers". Please add all required values to "layers" before '
+            'including them in "domain_contrasts".'
+        ),
     ):
         RATapi.Project(calculation=Calculations.Domains, domain_contrasts=RATapi.ClassList(test_contrast))
 
@@ -954,8 +971,10 @@ def test_check_allowed_values_not_on_list(test_value: str) -> None:
     allowed_values = ["Substrate Roughness"]
     with pytest.raises(
         ValueError,
-        match=f'The value "{test_value}" used in the "roughness" field at index 0 of "layers" must be defined in '
-        f'"parameters". Please add "{test_value}" to "parameters" before including it in "layers".',
+        match=re.escape(
+            f'The value "{test_value}" used in the "roughness" field of layers[0] must be defined in "parameters". '
+            f'Please add "{test_value}" to "parameters" before including it in "layers".'
+        ),
     ):
         project.check_allowed_values("layers", ["roughness"], allowed_values, allowed_values)
 
@@ -1004,9 +1023,11 @@ def test_check_allowed_background_resolution_values_not_on_constant_list(test_va
     )
     with pytest.raises(
         ValueError,
-        match=f'The value "{test_value}" used in the "source" field at index 0 of "backgrounds" must be defined '
-        f'in "background_parameters". Please add "{test_value}" to "background_parameters" before including '
-        f'it in "backgrounds".',
+        match=re.escape(
+            f'The value "{test_value}" used in the "source" field of backgrounds[0] must be defined in '
+            f'"background_parameters". Please add "{test_value}" to "background_parameters" before including it in '
+            f'"backgrounds".'
+        ),
     ):
         project.check_allowed_source(
             "backgrounds",
@@ -1029,8 +1050,10 @@ def test_check_allowed_background_resolution_values_on_data_list(test_value: str
     )
     with pytest.raises(
         ValueError,
-        match=f'The value "{test_value}" used in the "source" field at index 0 of "backgrounds" must be defined '
-        f'in "data". Please add "{test_value}" to "data" before including it in "backgrounds".',
+        match=re.escape(
+            f'The value "{test_value}" used in the "source" field of backgrounds[0] must be defined in "data". Please '
+            f'add "{test_value}" to "data" before including it in "backgrounds".'
+        ),
     ):
         project.check_allowed_source("backgrounds")
 
@@ -1068,9 +1091,10 @@ def test_check_allowed_contrast_model_not_on_list(test_values: list[str]) -> Non
     )
     with pytest.raises(
         ValueError,
-        match=f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field at index 0 of '
-        f'"contrasts" must be defined in "layers". Please add all required values to "layers" before '
-        f'including them in "contrasts".',
+        match=re.escape(
+            f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field of contrasts[0] must '
+            f'be defined in "layers". Please add all required values to "layers" before including them in "contrasts".'
+        ),
     ):
         project.check_contrast_model_allowed_values("contrasts", ["Test Layer"], ["Test Layer"], "layers")
 
@@ -1091,9 +1115,11 @@ def test_check_allowed_contrast_model_removed_from_list(test_values: list[str])
     )
     with pytest.raises(
         ValueError,
-        match=f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field at index 0 of '
-        f'"contrasts" must be defined in "layers". Please remove all unnecessary values from "model" before '
-        f"attempting to delete them.",
+        match=re.escape(
+            f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field of contrasts[0] must '
+            f'be defined in "layers". Please remove all unnecessary values from "model" before attempting to delete '
+            f"them."
+        ),
     ):
         project.check_contrast_model_allowed_values("contrasts", [], ["Test Layer", "Undefined Param"], "layers")
 
@@ -1187,10 +1213,12 @@ def test_wrap_set(test_project, class_list: str, model_type: str, field: str) ->
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
-        f'at index {index} of "{class_list}" must be defined in '
-        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
-        f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field of '
+            f'{class_list}[{index}] must be defined in "{RATapi.project.values_defined_in[class_list_str]}". Please '
+            f'add "undefined" to "{RATapi.project.values_defined_in[class_list_str]}" before including it in '
+            f'"{class_list}".'
+        ),
     ):
         test_attribute.set_fields(index, **{field: "undefined"})
 
@@ -1224,9 +1252,11 @@ def test_wrap_del(test_project, class_list: str, parameter: str, field: str) ->
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" '
-        f'field at index {sub_index} of "{sub_attribute_name}" must be defined in "{class_list}". '
-        f'Please remove "{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" field of '
+            f'{sub_attribute_name}[{sub_index}] must be defined in "{class_list}". Please remove "{parameter}" from '
+            f'"{sub_attribute_name}[{sub_index}].{field}" before attempting to delete it.'
+        ),
     ):
         del test_attribute[index]
 
@@ -1260,10 +1290,12 @@ def test_wrap_iadd(test_project, class_list: str, model_type: str, field: str, m
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
-        f'at index {len(test_attribute)} of "{class_list}" must be defined in '
-        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
-        f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" '
+            f"field of {class_list}[{len(test_attribute)}] must be defined in "
+            f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
+            f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".'
+        ),
     ):
         test_attribute += [input_model(**{**model_params, field: "undefined"})]
 
@@ -1298,10 +1330,12 @@ def test_wrap_append(test_project, class_list: str, model_type: str, field: str,
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}(" field '
-        f'at index {len(test_attribute)} of "){class_list}" must be defined in '
-        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
-        f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" '
+            f"field of {class_list}[{len(test_attribute)}] must be defined in "
+            f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
+            f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".'
+        ),
     ):
         test_attribute.append(input_model(**{**model_params, field: "undefined"}))
 
@@ -1336,10 +1370,12 @@ def test_wrap_insert(test_project, class_list: str, model_type: str, field: str,
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
-        f'at index {index} of "{class_list}" must be defined in '
-        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
-        f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" '
+            f"field of {class_list}[{index}] must be defined in "
+            f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
+            f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".'
+        ),
     ):
         test_attribute.insert(index, input_model(**{**model_params, field: "undefined"}))
 
@@ -1410,9 +1446,11 @@ def test_wrap_pop(test_project, class_list: str, parameter: str, field: str) ->
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" '
-        f'field at index {sub_index} of "{sub_attribute_name}" must be defined in "{class_list}". Please '
-        f'remove "{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" field of '
+            f'{sub_attribute_name}[{sub_index}] must be defined in "{class_list}". Please remove "{parameter}" from '
+            f'"{sub_attribute_name}[{sub_index}].{field}" before attempting to delete it.'
+        ),
     ):
         test_attribute.pop(index)
 
@@ -1445,9 +1483,11 @@ def test_wrap_remove(test_project, class_list: str, parameter: str, field: str)
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" '
-        f'field at index {sub_index} of "{sub_attribute_name}" must be defined in "{class_list}". Please '
-        f'remove "{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" field of '
+            f'{sub_attribute_name}[{sub_index}] must be defined in "{class_list}". Please remove "{parameter}" from '
+            f'"{sub_attribute_name}[{sub_index}].{field}" before attempting to delete it.'
+        ),
     ):
         test_attribute.remove(parameter)
 
@@ -1480,9 +1520,11 @@ def test_wrap_clear(test_project, class_list: str, parameter: str, field: str) -
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" '
-        f'field at index {sub_index} of "{sub_attribute_name}" must be defined in "{class_list}". Please '
-        f'remove "{parameter}" from "{sub_attribute_name}{sub_index}.{field}" before attempting to delete it.',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "{parameter}" used in the "{field}" field of '
+            f'{sub_attribute_name}[{sub_index}] must be defined in "{class_list}". Please remove "{parameter}" from '
+            f'"{sub_attribute_name}[{sub_index}].{field}" before attempting to delete it.'
+        ),
     ):
         test_attribute.clear()
 
@@ -1516,10 +1558,12 @@ def test_wrap_extend(test_project, class_list: str, model_type: str, field: str,
 
     with pytest.raises(
         pydantic.ValidationError,
-        match=f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" field '
-        f'at index {len(test_attribute)} of "{class_list}" must be defined in '
-        f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
-        f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".',
+        match=re.escape(
+            f'1 validation error for Project\n  Value error, The value "undefined" used in the "{field}" '
+            f"field of {class_list}[{len(test_attribute)}] must be defined in "
+            f'"{RATapi.project.values_defined_in[class_list_str]}". Please add "undefined" to '
+            f'"{RATapi.project.values_defined_in[class_list_str]}" before including it in "{class_list}".'
+        ),
     ):
         test_attribute.extend([input_model(**{**model_params, field: "undefined"})])
 

From bb5d450f056042626fc4e68e8a6e9f4483526360 Mon Sep 17 00:00:00 2001
From: Paul Sharp <44529197+DrPaulSharp@users.noreply.github.com>
Date: Wed, 23 Apr 2025 14:37:30 +0100
Subject: [PATCH 5/5] Improves error message for contrast model

---
 RATapi/project.py     | 16 +++++++++-------
 tests/test_project.py | 39 ++++++++++++++++++++++++---------------
 2 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/RATapi/project.py b/RATapi/project.py
index f2e13a6f..ae2ed2cc 100644
--- a/RATapi/project.py
+++ b/RATapi/project.py
@@ -767,18 +767,20 @@ def check_contrast_model_allowed_values(
         """
         class_list = getattr(self, contrast_attribute)
         for index, contrast in enumerate(class_list):
-            if (model_values := contrast.model) and not all(value in allowed_values for value in model_values):
+            if (model_values := contrast.model) and (missing_values := list(set(model_values) - set(allowed_values))):
                 if all(value in previous_values for value in model_values):
                     raise ValueError(
-                        f'The values: "{", ".join(str(i) for i in model_values)}" used in the "model" field of '
-                        f'{contrast_attribute}[{index}] must be defined in "{allowed_field}". Please remove '
-                        f'all unnecessary values from "model" before attempting to delete them.',
+                        f"The value{'s' if len(missing_values) > 1 else ''}: "
+                        f'"{", ".join(str(i) for i in missing_values)}" used in the "model" field of '
+                        f'{contrast_attribute}[{index}] must be defined in "{allowed_field}". Please remove all '
+                        f'unnecessary values from "model" before attempting to delete them.',
                     )
                 else:
                     raise ValueError(
-                        f'The values: "{", ".join(str(i) for i in model_values)}" used in the "model" field of '
-                        f'{contrast_attribute}[{index}] must be defined in "{allowed_field}". Please add '
-                        f'all required values to "{allowed_field}" before including them in "{contrast_attribute}".',
+                        f"The value{'s' if len(missing_values) > 1 else ''}: "
+                        f'"{", ".join(str(i) for i in missing_values)}" used in the "model" field of '
+                        f'{contrast_attribute}[{index}] must be defined in "{allowed_field}". Please add all '
+                        f'required values to "{allowed_field}" before including them in "{contrast_attribute}".',
                     )
 
     def get_contrast_model_field(self):
diff --git a/tests/test_project.py b/tests/test_project.py
index 82d1a8f7..e82a84da 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -876,12 +876,13 @@ def test_allowed_contrast_models(
     """If any value in the model field of the contrasts is set to a value not specified in the appropriate part of the
     project, we should raise a ValidationError.
     """
+    missing_values = list(set(test_contrast.model))
     with pytest.raises(
         pydantic.ValidationError,
         match=re.escape(
-            f'1 validation error for Project\n  Value error, The values: "{", ".join(test_contrast.model)}" used in '
-            f'the "model" field of contrasts[0] must be defined in "{field_name}". Please add all required values to '
-            f'"{field_name}" before including them in "contrasts".'
+            f"1 validation error for Project\n  Value error, The value{'s' if len(missing_values) > 1 else ''}: "
+            f'"{", ".join(missing_values)}" used in the "model" field of contrasts[0] must be defined in '
+            f'"{field_name}". Please add all required values to "{field_name}" before including them in "contrasts".'
         ),
     ):
         RATapi.Project(calculation=input_calc, model=input_model, contrasts=RATapi.ClassList(test_contrast))
@@ -895,7 +896,7 @@ def test_allowed_domain_contrast_models() -> None:
     with pytest.raises(
         pydantic.ValidationError,
         match=re.escape(
-            '1 validation error for Project\n  Value error, The values: "undefined" used in the "model" field of '
+            '1 validation error for Project\n  Value error, The value: "undefined" used in the "model" field of '
             'domain_contrasts[0] must be defined in "layers". Please add all required values to "layers" before '
             'including them in "domain_contrasts".'
         ),
@@ -1078,8 +1079,9 @@ def test_check_contrast_model_allowed_values(test_values: list[str]) -> None:
 @pytest.mark.parametrize(
     "test_values",
     [
-        ["Undefined Param"],
-        ["Test Layer", "Undefined Param"],
+        ["Undefined Param 1"],
+        ["Test Layer", "Undefined Param 1"],
+        ["Undefined Param 1 ", "Test Layer", "Undefined Param 2"],
     ],
 )
 def test_check_allowed_contrast_model_not_on_list(test_values: list[str]) -> None:
@@ -1089,21 +1091,25 @@ def test_check_allowed_contrast_model_not_on_list(test_values: list[str]) -> Non
     project = RATapi.Project.model_construct(
         contrasts=RATapi.ClassList(RATapi.models.Contrast(name="Test Contrast", model=test_values)),
     )
+    allowed_values = ["Test Layer"]
+    missing_values = list(set(test_values) - set(allowed_values))
     with pytest.raises(
         ValueError,
         match=re.escape(
-            f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field of contrasts[0] must '
-            f'be defined in "layers". Please add all required values to "layers" before including them in "contrasts".'
+            f'The value{"s" if len(missing_values) > 1 else ""}: "{", ".join(str(i) for i in missing_values)}" used '
+            f'in the "model" field of contrasts[0] must be defined in "layers". Please add all required values to '
+            f'"layers" before including them in "contrasts".'
         ),
     ):
-        project.check_contrast_model_allowed_values("contrasts", ["Test Layer"], ["Test Layer"], "layers")
+        project.check_contrast_model_allowed_values("contrasts", allowed_values, allowed_values, "layers")
 
 
 @pytest.mark.parametrize(
     "test_values",
     [
-        ["Test Layer"],
-        ["Test Layer", "Undefined Param"],
+        ["Undefined Param 1"],
+        ["Test Layer", "Undefined Param 1"],
+        ["Undefined Param 1", "Test Layer", "Undefined Param 2"],
     ],
 )
 def test_check_allowed_contrast_model_removed_from_list(test_values: list[str]) -> None:
@@ -1113,15 +1119,18 @@ def test_check_allowed_contrast_model_removed_from_list(test_values: list[str])
     project = RATapi.Project.model_construct(
         contrasts=RATapi.ClassList(RATapi.models.Contrast(name="Test Contrast", model=test_values)),
     )
+    previous_values = ["Test Layer", "Undefined Param 1", "Undefined Param 2"]
+    allowed_values = ["Test Layer"]
+    missing_values = list(set(test_values) - set(allowed_values))
     with pytest.raises(
         ValueError,
         match=re.escape(
-            f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field of contrasts[0] must '
-            f'be defined in "layers". Please remove all unnecessary values from "model" before attempting to delete '
-            f"them."
+            f'The value{"s" if len(missing_values) > 1 else ""}: "{", ".join(str(i) for i in missing_values)}" used '
+            f'in the "model" field of contrasts[0] must be defined in "layers". Please remove all unnecessary values '
+            f'from "model" before attempting to delete them.'
         ),
     ):
-        project.check_contrast_model_allowed_values("contrasts", [], ["Test Layer", "Undefined Param"], "layers")
+        project.check_contrast_model_allowed_values("contrasts", allowed_values, previous_values, "layers")
 
 
 @pytest.mark.parametrize(