Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DrPaulSharp merged 3 commits into from
Mar 27, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5421a7c
Changes model warnings to errors
DrPaulSharp Mar 26, 2025
5965028
Adds clause to "set_fields" to address bug
DrPaulSharp Mar 26, 2025
a3ed1ce
Updates tests
DrPaulSharp Mar 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions RATapi/classlist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
22 changes: 10 additions & 12 deletions RATapi/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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
46 changes: 37 additions & 9 deletions tests/test_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
50 changes: 0 additions & 50 deletions tests/test_project.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down