Skip to content

Commit bb5d450

Browse files
committed
Improves error message for contrast model
1 parent b054642 commit bb5d450

File tree

2 files changed

+33
-22
lines changed

2 files changed

+33
-22
lines changed

RATapi/project.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -767,18 +767,20 @@ def check_contrast_model_allowed_values(
767767
"""
768768
class_list = getattr(self, contrast_attribute)
769769
for index, contrast in enumerate(class_list):
770-
if (model_values := contrast.model) and not all(value in allowed_values for value in model_values):
770+
if (model_values := contrast.model) and (missing_values := list(set(model_values) - set(allowed_values))):
771771
if all(value in previous_values for value in model_values):
772772
raise ValueError(
773-
f'The values: "{", ".join(str(i) for i in model_values)}" used in the "model" field of '
774-
f'{contrast_attribute}[{index}] must be defined in "{allowed_field}". Please remove '
775-
f'all unnecessary values from "model" before attempting to delete them.',
773+
f"The value{'s' if len(missing_values) > 1 else ''}: "
774+
f'"{", ".join(str(i) for i in missing_values)}" used in the "model" field of '
775+
f'{contrast_attribute}[{index}] must be defined in "{allowed_field}". Please remove all '
776+
f'unnecessary values from "model" before attempting to delete them.',
776777
)
777778
else:
778779
raise ValueError(
779-
f'The values: "{", ".join(str(i) for i in model_values)}" used in the "model" field of '
780-
f'{contrast_attribute}[{index}] must be defined in "{allowed_field}". Please add '
781-
f'all required values to "{allowed_field}" before including them in "{contrast_attribute}".',
780+
f"The value{'s' if len(missing_values) > 1 else ''}: "
781+
f'"{", ".join(str(i) for i in missing_values)}" used in the "model" field of '
782+
f'{contrast_attribute}[{index}] must be defined in "{allowed_field}". Please add all '
783+
f'required values to "{allowed_field}" before including them in "{contrast_attribute}".',
782784
)
783785

784786
def get_contrast_model_field(self):

tests/test_project.py

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -876,12 +876,13 @@ def test_allowed_contrast_models(
876876
"""If any value in the model field of the contrasts is set to a value not specified in the appropriate part of the
877877
project, we should raise a ValidationError.
878878
"""
879+
missing_values = list(set(test_contrast.model))
879880
with pytest.raises(
880881
pydantic.ValidationError,
881882
match=re.escape(
882-
f'1 validation error for Project\n Value error, The values: "{", ".join(test_contrast.model)}" used in '
883-
f'the "model" field of contrasts[0] must be defined in "{field_name}". Please add all required values to '
884-
f'"{field_name}" before including them in "contrasts".'
883+
f"1 validation error for Project\n Value error, The value{'s' if len(missing_values) > 1 else ''}: "
884+
f'"{", ".join(missing_values)}" used in the "model" field of contrasts[0] must be defined in '
885+
f'"{field_name}". Please add all required values to "{field_name}" before including them in "contrasts".'
885886
),
886887
):
887888
RATapi.Project(calculation=input_calc, model=input_model, contrasts=RATapi.ClassList(test_contrast))
@@ -895,7 +896,7 @@ def test_allowed_domain_contrast_models() -> None:
895896
with pytest.raises(
896897
pydantic.ValidationError,
897898
match=re.escape(
898-
'1 validation error for Project\n Value error, The values: "undefined" used in the "model" field of '
899+
'1 validation error for Project\n Value error, The value: "undefined" used in the "model" field of '
899900
'domain_contrasts[0] must be defined in "layers". Please add all required values to "layers" before '
900901
'including them in "domain_contrasts".'
901902
),
@@ -1078,8 +1079,9 @@ def test_check_contrast_model_allowed_values(test_values: list[str]) -> None:
10781079
@pytest.mark.parametrize(
10791080
"test_values",
10801081
[
1081-
["Undefined Param"],
1082-
["Test Layer", "Undefined Param"],
1082+
["Undefined Param 1"],
1083+
["Test Layer", "Undefined Param 1"],
1084+
["Undefined Param 1 ", "Test Layer", "Undefined Param 2"],
10831085
],
10841086
)
10851087
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
10891091
project = RATapi.Project.model_construct(
10901092
contrasts=RATapi.ClassList(RATapi.models.Contrast(name="Test Contrast", model=test_values)),
10911093
)
1094+
allowed_values = ["Test Layer"]
1095+
missing_values = list(set(test_values) - set(allowed_values))
10921096
with pytest.raises(
10931097
ValueError,
10941098
match=re.escape(
1095-
f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field of contrasts[0] must '
1096-
f'be defined in "layers". Please add all required values to "layers" before including them in "contrasts".'
1099+
f'The value{"s" if len(missing_values) > 1 else ""}: "{", ".join(str(i) for i in missing_values)}" used '
1100+
f'in the "model" field of contrasts[0] must be defined in "layers". Please add all required values to '
1101+
f'"layers" before including them in "contrasts".'
10971102
),
10981103
):
1099-
project.check_contrast_model_allowed_values("contrasts", ["Test Layer"], ["Test Layer"], "layers")
1104+
project.check_contrast_model_allowed_values("contrasts", allowed_values, allowed_values, "layers")
11001105

11011106

11021107
@pytest.mark.parametrize(
11031108
"test_values",
11041109
[
1105-
["Test Layer"],
1106-
["Test Layer", "Undefined Param"],
1110+
["Undefined Param 1"],
1111+
["Test Layer", "Undefined Param 1"],
1112+
["Undefined Param 1", "Test Layer", "Undefined Param 2"],
11071113
],
11081114
)
11091115
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])
11131119
project = RATapi.Project.model_construct(
11141120
contrasts=RATapi.ClassList(RATapi.models.Contrast(name="Test Contrast", model=test_values)),
11151121
)
1122+
previous_values = ["Test Layer", "Undefined Param 1", "Undefined Param 2"]
1123+
allowed_values = ["Test Layer"]
1124+
missing_values = list(set(test_values) - set(allowed_values))
11161125
with pytest.raises(
11171126
ValueError,
11181127
match=re.escape(
1119-
f'The values: "{", ".join(str(i) for i in test_values)}" used in the "model" field of contrasts[0] must '
1120-
f'be defined in "layers". Please remove all unnecessary values from "model" before attempting to delete '
1121-
f"them."
1128+
f'The value{"s" if len(missing_values) > 1 else ""}: "{", ".join(str(i) for i in missing_values)}" used '
1129+
f'in the "model" field of contrasts[0] must be defined in "layers". Please remove all unnecessary values '
1130+
f'from "model" before attempting to delete them.'
11221131
),
11231132
):
1124-
project.check_contrast_model_allowed_values("contrasts", [], ["Test Layer", "Undefined Param"], "layers")
1133+
project.check_contrast_model_allowed_values("contrasts", allowed_values, previous_values, "layers")
11251134

11261135

11271136
@pytest.mark.parametrize(

0 commit comments

Comments
 (0)