Fixes bugs in model validation when using "set_fields" (#144)

* Changes model warnings to errors

* Adds clause to "set_fields" to address bug

* Updates tests

Author: DrPaulSharp

RATapi/classlist.py

@@ -318,6 +318,11 @@ def set_fields(self, index: Union[int, slice, str, T], **kwargs) -> None:
         if isinstance(index, (str, self._class_handle)):
             index = self.index(index)
 
+        # Prioritise changing language to avoid CustomFile validator bug
+        value = kwargs.pop("language", None)
+        if value is not None:
+            kwargs = {"language": value, **kwargs}
+
         if importlib.util.find_spec("pydantic"):
             # Pydantic is installed, so set up a context manager that will
             # suppress custom validation errors until all fields have been set.

RATapi/models.py

@@ -119,8 +119,8 @@ class Background(Signal):
     name: str = Field(default_factory=lambda: f"New Background {next(background_number)}", min_length=1)
 
     @model_validator(mode="after")
-    def warn_parameters(self):
-        """Raise a warning if the parameters given are not expected for the given type."""
+    def check_unsupported_parameters(self):
+        """Raise an error if the parameters given are not supported for the given type."""
         if self.type == TypeOptions.Constant:
             expected_empty_fields = ["value_1", "value_2", "value_3", "value_4", "value_5"]
         elif self.type == TypeOptions.Data:
@@ -130,10 +130,9 @@ def warn_parameters(self):
 
         non_empty_fields = [v for v in expected_empty_fields if getattr(self, v) != ""]
         if non_empty_fields:
-            warnings.warn(
-                "The following values are not recognised by this background type and will be ignored: "
-                f"{', '.join(non_empty_fields)}",
-                stacklevel=2,
+            raise ValueError(
+                f'The following values are not supported by the "{self.type}" Background type: '
+                f"{', '.join(non_empty_fields)}"
             )
 
         return self
@@ -630,8 +629,8 @@ def validate_unimplemented_resolutions(cls, type: TypeOptions):
         return type
 
     @model_validator(mode="after")
-    def warn_parameters(self):
-        """Raise a warning if the parameters given are not expected for the given type."""
+    def check_unsupported_parameters(self):
+        """Raise an error if the parameters given are not supported for the given type."""
         if self.type == TypeOptions.Constant:
             expected_empty_fields = ["value_1", "value_2", "value_3", "value_4", "value_5"]
         elif self.type == TypeOptions.Data:
@@ -641,10 +640,9 @@ def warn_parameters(self):
 
         non_empty_fields = [v for v in expected_empty_fields if getattr(self, v) != ""]
         if non_empty_fields:
-            warnings.warn(
-                "The following values are not recognised by this resolution type and will be ignored: "
-                f"{', '.join(non_empty_fields)}",
-                stacklevel=2,
+            raise ValueError(
+                f'The following values are not supported by the "{self.type}" Resolution type: '
+                f"{', '.join(non_empty_fields)}"
             )
 
         return self

tests/test_models.py

@@ -334,22 +334,50 @@ def test_contrast_bad_ratio():
         RATapi.models.Contrast(name="My Contrast", domain_ratio="bad ratio")
 
 
-@pytest.mark.parametrize("model", [RATapi.models.Background, RATapi.models.Resolution])
-@pytest.mark.filterwarnings("ignore:The following values are not recognised by this*:UserWarning")
-def test_type_change_clear(model):
+@pytest.mark.parametrize(
+    ["model", "type", "values"],
+    [
+        (RATapi.models.Background, "function", ["val1", "val2", "val3", "val4", "val5"]),
+        (RATapi.models.Resolution, "constant", ["", "", "", "", ""]),
+    ],
+)
+def test_type_change_clear(model, type, values):
     """If the type of a background or resolution is changed, it should wipe the other fields and warn the user."""
     model_instance = model(
         name="Test",
-        type="constant",
+        type=type,
         source="src",
-        value_1="val1",
-        value_2="val2",
-        value_3="val3",
-        value_4="val4",
-        value_5="val5",
+        value_1=values[0],
+        value_2=values[1],
+        value_3=values[2],
+        value_4=values[3],
+        value_5=values[4],
     )
 
     with pytest.warns(UserWarning, match="Changing the type of Test clears its source and value fields."):
         model_instance.type = "data"
     for attr in ["source", "value_1", "value_2", "value_3", "value_4", "value_5"]:
         assert getattr(model_instance, attr) == ""
+
+
+@pytest.mark.parametrize(
+    ["model", "signal_type", "values"],
+    [
+        (RATapi.models.Background, "constant", ["value_1", "value_2", "value_3", "value_4", "value_5"]),
+        (RATapi.models.Background, "data", ["value_2", "value_3", "value_4", "value_5"]),
+        (RATapi.models.Resolution, "constant", ["value_1", "value_2", "value_3", "value_4", "value_5"]),
+        (RATapi.models.Resolution, "data", ["value_1", "value_2", "value_3", "value_4", "value_5"]),
+    ],
+)
+def test_unsupported_parameters_error(model, signal_type, values):
+    """If a value is inputted for an unsupported field for a particular type of background or resolution then we should
+    raise an error."""
+    for value in values:
+        with pytest.raises(
+            pydantic.ValidationError,
+            match=(
+                f"1 validation error for {model.__name__}\n  Value error, The following values are not supported"
+                f' by the "{signal_type}" {model.__name__} type: {value}'
+            ),
+        ):
+            model(**{"type": signal_type, value: "unsupported"})

tests/test_project.py

@@ -1137,17 +1137,7 @@ def test_write_script_wrong_extension(test_project, extension: str) -> None:
     ["class_list", "model_type", "field"],
     [
         ("backgrounds", "constant", "source"),
-        ("backgrounds", "", "value_1"),
-        ("backgrounds", "", "value_2"),
-        ("backgrounds", "", "value_3"),
-        ("backgrounds", "", "value_4"),
-        ("backgrounds", "", "value_5"),
         ("resolutions", "constant", "source"),
-        ("resolutions", "", "value_1"),
-        ("resolutions", "", "value_2"),
-        ("resolutions", "", "value_3"),
-        ("resolutions", "", "value_4"),
-        ("resolutions", "", "value_5"),
         ("layers", "", "thickness"),
         ("layers", "", "SLD"),
         ("layers", "", "roughness"),
@@ -1216,17 +1206,7 @@ def test_wrap_del(test_project, class_list: str, parameter: str, field: str) ->
     ["class_list", "model_type", "field", "model_params"],
     [
         ("backgrounds", "constant", "source", {}),
-        ("backgrounds", "", "value_1", {}),
-        ("backgrounds", "", "value_2", {}),
-        ("backgrounds", "", "value_3", {}),
-        ("backgrounds", "", "value_4", {}),
-        ("backgrounds", "", "value_5", {}),
         ("resolutions", "constant", "source", {}),
-        ("resolutions", "", "value_1", {}),
-        ("resolutions", "", "value_2", {}),
-        ("resolutions", "", "value_3", {}),
-        ("resolutions", "", "value_4", {}),
-        ("resolutions", "", "value_5", {}),
         ("layers", "", "thickness", layer_params),
         ("layers", "", "SLD", layer_params),
         ("layers", "", "roughness", layer_params),
@@ -1263,17 +1243,7 @@ def test_wrap_iadd(test_project, class_list: str, model_type: str, field: str, m
     ["class_list", "model_type", "field", "model_params"],
     [
         ("backgrounds", "constant", "source", {}),
-        ("backgrounds", "", "value_1", {}),
-        ("backgrounds", "", "value_2", {}),
-        ("backgrounds", "", "value_3", {}),
-        ("backgrounds", "", "value_4", {}),
-        ("backgrounds", "", "value_5", {}),
         ("resolutions", "constant", "source", {}),
-        ("resolutions", "", "value_1", {}),
-        ("resolutions", "", "value_2", {}),
-        ("resolutions", "", "value_3", {}),
-        ("resolutions", "", "value_4", {}),
-        ("resolutions", "", "value_5", {}),
         ("layers", "", "thickness", layer_params),
         ("layers", "", "SLD", layer_params),
         ("layers", "", "roughness", layer_params),
@@ -1311,17 +1281,7 @@ def test_wrap_append(test_project, class_list: str, model_type: str, field: str,
     ["class_list", "model_type", "field", "model_params"],
     [
         ("backgrounds", "constant", "source", {}),
-        ("backgrounds", "", "value_1", {}),
-        ("backgrounds", "", "value_2", {}),
-        ("backgrounds", "", "value_3", {}),
-        ("backgrounds", "", "value_4", {}),
-        ("backgrounds", "", "value_5", {}),
         ("resolutions", "constant", "source", {}),
-        ("resolutions", "", "value_1", {}),
-        ("resolutions", "", "value_2", {}),
-        ("resolutions", "", "value_3", {}),
-        ("resolutions", "", "value_4", {}),
-        ("resolutions", "", "value_5", {}),
         ("layers", "", "thickness", layer_params),
         ("layers", "", "SLD", layer_params),
         ("layers", "", "roughness", layer_params),
@@ -1492,17 +1452,7 @@ def test_wrap_clear(test_project, class_list: str, parameter: str, field: str) -
     ["class_list", "model_type", "field", "model_params"],
     [
         ("backgrounds", "constant", "source", {}),
-        ("backgrounds", "", "value_1", {}),
-        ("backgrounds", "", "value_2", {}),
-        ("backgrounds", "", "value_3", {}),
-        ("backgrounds", "", "value_4", {}),
-        ("backgrounds", "", "value_5", {}),
         ("resolutions", "constant", "source", {}),
-        ("resolutions", "", "value_1", {}),
-        ("resolutions", "", "value_2", {}),
-        ("resolutions", "", "value_3", {}),
-        ("resolutions", "", "value_4", {}),
-        ("resolutions", "", "value_5", {}),
         ("layers", "", "thickness", layer_params),
         ("layers", "", "SLD", layer_params),
         ("layers", "", "roughness", layer_params),