Skip to content

Commit 5c489d1

Browse files
VisLabVisLab
committed
Fixed validation for DefinitionDict
1 parent 5c95a01 commit 5c489d1

9 files changed

Lines changed: 114 additions & 86 deletions

File tree

‎hed/models/definition_dict.py‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
""" Definition handler class. """
2-
from typing import Union
2+
from __future__ import annotations
3+
from typing import Union, TYPE_CHECKING
34

45
from hed.models.definition_entry import DefinitionEntry
56
from hed.models.hed_string import HedString
67
from hed.errors.error_types import DefinitionErrors
7-
from hed.errors.error_reporter import ErrorHandler
8+
from hed.errors.error_reporter import ErrorHandler, check_for_any_errors
89
from hed.models.model_constants import DefTagNames
910
from hed.schema.hed_schema_constants import HedKey
1011

11-
1212
class DefinitionDict:
1313
""" Gathers definitions from a single source. """
1414

@@ -124,6 +124,17 @@ def check_for_definitions(self, hed_string_obj, error_handler=None) -> list[dict
124124
list[dict]: List of issues encountered in checking for definitions. Each issue is a dictionary.
125125
"""
126126
def_issues = []
127+
if "definition" not in hed_string_obj._hed_string.casefold():
128+
# Check if the casefold of hed_string contains "definition" so checking is applicable
129+
return def_issues
130+
if hed_string_obj._schema:
131+
from hed.validator.hed_validator import HedValidator
132+
validator = HedValidator(hed_string_obj._schema, def_dicts=self, definitions_allowed=True)
133+
issues = validator.validate(hed_string_obj, allow_placeholders=True, error_handler=error_handler)
134+
if check_for_any_errors(issues):
135+
return issues
136+
else:
137+
def_issues += issues
127138
for definition_tag, group in hed_string_obj.find_top_level_tags(anchor_tags={DefTagNames.DEFINITION_KEY}):
128139
group_tag, new_def_issues = self._find_group(definition_tag, group, error_handler)
129140
def_tag_name, def_takes_value = self._strip_value_placeholder(definition_tag.extension)

‎tests/data/sidecar_tests/both_types_events_with_defs.json‎

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,38 @@
1-
{
2-
"trial_type": {
3-
"LongName": "Event category",
4-
"Description": "Indicator of type of action that is expected",
5-
"Levels": {
6-
"go": "A red square is displayed to indicate starting",
7-
"stop": "A blue square is displayed to indicate stopping"
8-
},
9-
"HED": {
10-
"go": "Item/ItemTag1",
11-
"stop": "Item/ItemTag2"
12-
}
13-
},
14-
"response_time": {
15-
"LongName": "Response time after stimulus",
16-
"Description": "Time from stimulus presentation until subject presses button",
17-
"Units": "ms",
18-
"HED": "Def/JsonFileDef/#, Action/Button press"
19-
},
20-
"stim_file": {
21-
"LongName": "Stimulus file name",
22-
"Description": "Relative path of the stimulus image file",
23-
"HED": "Age/#, (Definition/JsonFileDef2/#, (Age/#,Item/JsonDef2)), (Definition/JsonFileDef3/#, (Age/#))"
24-
},
25-
"takes_value_def": {
26-
"LongName": "Def with a takes value tag",
27-
"Description": "Relative path of the stimulus image file",
28-
"HED": "Age/#, (Definition/TakesValueDef/#, (Age/#))"
29-
},
30-
"unit_class_def": {
31-
"LongName": "Def with a value class",
32-
"Description": "Relative path of the stimulus image file",
33-
"HED": "Age/#, (Definition/ValueClassDef/#, (Acceleration/#))"
34-
},
35-
"Defs": {
36-
"HED": "(Definition/JsonFileDef/#, (Acceleration/#,Item/JsonDef1))"
37-
}
1+
{
2+
"trial_type": {
3+
"LongName": "Event category",
4+
"Description": "Indicator of type of action that is expected",
5+
"Levels": {
6+
"go": "A red square is displayed to indicate starting",
7+
"stop": "A blue square is displayed to indicate stopping"
8+
},
9+
"HED": {
10+
"go": "Item/ItemTag1",
11+
"stop": "Item/ItemTag2"
12+
}
13+
},
14+
"response_time": {
15+
"LongName": "Response time after stimulus",
16+
"Description": "Time from stimulus presentation until subject presses button",
17+
"Units": "ms",
18+
"HED": "Def/JsonFileDef/#, Action/Button press"
19+
},
20+
"stim_file": {
21+
"LongName": "Stimulus file name",
22+
"Description": "Relative path of the stimulus image file",
23+
"HED": "(Definition/JsonFileDef2/#, (Age/#,Item/JsonDef2)), (Definition/JsonFileDef3/#, (Age/#))"
24+
},
25+
"takes_value_def": {
26+
"LongName": "Def with a takes value tag",
27+
"Description": "Relative path of the stimulus image file",
28+
"HED": "(Definition/TakesValueDef/#, (Age/#))"
29+
},
30+
"unit_class_def": {
31+
"LongName": "Def with a value class",
32+
"Description": "Relative path of the stimulus image file",
33+
"HED": "Age/#, (Definition/ValueClassDef/#, (Acceleration/#))"
34+
},
35+
"Defs": {
36+
"HED": "(Definition/JsonFileDef/#, (Acceleration/#,Item/JsonDef1))"
37+
}
3838
}

‎tests/data/validator_tests/bids_events.json‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,16 @@
8181
},
8282
"defs": {
8383
"HED": {
84-
"1": "Description/BCI acts randomly, (Definition/Random-selection, (Condition-variable, (Random, Predict))), Def/Random-selection",
85-
"2": "Description/BCI was trained on data from stage 1., (Definition/Trained-on-random, (Condition-variable)), Def/Trained-on-random",
86-
"3": "Description/BCI was trained on data from stages 1 and 2., (Definition/Trained-on-all, (Condition-variable)), Def/Trained-on-all"
84+
"1": "Description/BCI acts randomly, Def/Random-selection",
85+
"2": "Description/BCI was trained on data from stage 1., Def/Trained-on-random",
86+
"3": "Description/BCI was trained on data from stages 1 and 2., Def/Trained-on-all"
87+
}
88+
},
89+
"defs_actual": {
90+
"HED":{
91+
"1": "(Definition/Random-selection, (Condition-variable, (Random, Predict)))",
92+
"2": "(Definition/Trained-on-random, (Condition-variable))",
93+
"3": "(Definition/Trained-on-all, (Condition-variable))"
8794
}
8895
}
8996
}

‎tests/data/validator_tests/bids_events_bad_defs2.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@
4141
"3": "Stage 3. BCI was trained on data from stages 1 and 2."
4242
},
4343
"HED": {
44-
"1": "Description/BCI acts randomly, (Definition/Random-selection, (Experimental-condition, (Random, Predict))), Def/Random-selection/InvalidPlaceholder",
45-
"2": "Description/BCI was trained on data from stage 1., (Definition/Trained-on-random, (Experimental-condition)), Def/Trained-on-random",
46-
"3": "Description/BCI was trained on data from stages 1 and 2."
44+
"1": "(Definition/Random-selection, (Condition-variable/random-select, (Random, Predict)))",
45+
"2": "(Definition/Trained-on-random, (Condition-variable/trained))",
46+
"3": "(Definition/NextOne)"
4747
}
4848
},
4949
"trial": {

‎tests/models/test_definition_dict.py‎

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import unittest
22
from hed.models.definition_dict import DefinitionDict
3-
from hed.errors import ErrorHandler, DefinitionErrors
3+
from hed.errors import ErrorHandler, DefinitionErrors, ValidationErrors, ErrorSeverity
44
from hed.models.hed_string import HedString
55
from hed import HedTag
66
from hed import load_schema_version
@@ -15,17 +15,18 @@ def setUpClass(cls):
1515
def check_def_base(self, test_strings, expected_issues):
1616
for test_key in test_strings:
1717
def_dict = DefinitionDict()
18+
def_dict.add_definitions("(Definition/OkayDef, (White))", self.hed_schema)
1819
hed_string_obj = HedString(test_strings[test_key], self.hed_schema)
1920
test_issues = def_dict.check_for_definitions(hed_string_obj)
21+
issues = [issue for issue in test_issues if issue['severity'] < ErrorSeverity.WARNING]
2022
expected_params = expected_issues[test_key]
2123
expected_issue = self.format_errors_fully(ErrorHandler(), hed_string=hed_string_obj,
2224
params=expected_params)
23-
self.assertCountEqual(test_issues, expected_issue, HedString(test_strings[test_key], self.hed_schema))
24-
25+
self.assertCountEqual(issues, expected_issue, HedString(test_strings[test_key], self.hed_schema))
2526

2627
class TestDefinitionDict(TestDefBase):
27-
def_contents_string = "(Item/TestDef1,Item/TestDef2)"
28-
def_contents_string2 = "(Item/TestDef3,Item/TestDef4)"
28+
def_contents_string = "(Item,Red)"
29+
def_contents_string2 = "(Property, Blue)"
2930
basic_definition_string = f"(Definition/TestDef,{def_contents_string})"
3031
label_def_string = "def/TestDef"
3132
expanded_def_string = f"(Def-expand/TestDef,{def_contents_string})"
@@ -51,7 +52,7 @@ def test_check_for_definitions_placeholder(self):
5152
new_def_count = len(def_dict.defs)
5253
self.assertGreater(new_def_count, original_def_count)
5354

54-
placeholder_invalid_def_contents = "(Age/#,Item/TestDef2/#)"
55+
placeholder_invalid_def_contents = "(Age/#,Item-count/#)"
5556
placeholder_invalid_def_string = f"(Definition/TestDefPlaceholder/#,{placeholder_invalid_def_contents})"
5657

5758
def test_definitions(self):
@@ -62,17 +63,17 @@ def test_definitions(self):
6263
'twoDefTags': f"(Definition/InvalidDef1,Definition/InvalidDef2,{self.def_contents_string})",
6364
'twoGroupTags': f"(Definition/InvalidDef1,{self.def_contents_string},{self.def_contents_string2})",
6465
'extraValidTags': "(Definition/InvalidDefA, Red, Blue)",
65-
'extraOtherTags': "(Definition/InvalidDef1, InvalidContents)",
66+
'extraOtherTags': "(Definition/InvalidDef1, Black)",
6667
'duplicateDef': (f"(Definition/Def1, {self.def_contents_string}), "
67-
f"(Definition/Def1, {self.def_contents_string})"),
68+
f"(Definition/Def1, (Green))"),
6869
'duplicateDef2': (f"(Definition/Def1, {self.def_contents_string}), "
6970
f"(Definition/Def1/#, {self.placeholder_def_contents})"),
7071
'defTooManyPlaceholders': self.placeholder_invalid_def_string,
7172
'invalidPlaceholder': f"(Definition/InvalidDef1/InvalidPlaceholder, {self.def_contents_string})",
7273
'invalidPlaceholderExtension':
7374
f"(Definition/InvalidDef1/this-part-is-not-allowed/#, {self.def_contents_string})",
74-
'defInGroup': "(Definition/ValidDefName, (Def/ImproperlyPlacedDef))",
75-
'defExpandInGroup': "(Definition/ValidDefName, (Def-expand/ImproperlyPlacedDef, (ImproperContents)))",
75+
'defInGroup': "(Definition/ValidDefName, (Def/OkayDef))",
76+
'defExpandInGroup': "(Definition/ValidDefName, (Def-expand/OkayDef, (White)))",
7677
'doublePoundSignPlaceholder': f"(Definition/InvalidDef/##, {self.placeholder_def_contents})",
7778
'doublePoundSignDiffPlaceholder': "(Definition/InvalidDef/#, (Age/##,Item/TestDef2))",
7879
'placeholdersWrongSpot': "(Definition/InvalidDef/#, (Age/#,Item/TestDef2))",
@@ -83,30 +84,31 @@ def test_definitions(self):
8384
'placeholderWrongSpot': self.format_error(DefinitionErrors.NO_DEFINITION_CONTENTS, "InvalidDef1#") +
8485
self.format_error(DefinitionErrors.INVALID_DEFINITION_EXTENSION,
8586
tag=0, def_name="InvalidDef1#"),
86-
'twoDefTags': self.format_error(DefinitionErrors.WRONG_NUMBER_TAGS, "InvalidDef1",
87-
["Definition/InvalidDef2"]),
88-
'twoGroupTags': self.format_error(DefinitionErrors.WRONG_NUMBER_GROUPS, "InvalidDef1",
89-
[self.def_contents_string, self.def_contents_string2]),
90-
'extraValidTags': self.format_error(DefinitionErrors.WRONG_NUMBER_TAGS, "InvalidDefA",
91-
["Red", "Blue"]),
92-
'extraOtherTags': self.format_error(DefinitionErrors.WRONG_NUMBER_TAGS, "InvalidDef1",
93-
["InvalidContents"]),
87+
'twoDefTags': self.format_error(ValidationErrors.HED_RESERVED_TAG_REPEATED, "Definition/InvalidDef2",
88+
"(Definition/InvalidDef1,Definition/InvalidDef2,(Item,Red))"),
89+
'twoGroupTags': self.format_error(ValidationErrors.HED_RESERVED_TAG_GROUP_ERROR,
90+
"(Definition/InvalidDef1,(Item,Red),(Property,Blue))", 2,
91+
["Definition/InvalidDef1"]),
92+
'extraValidTags': self.format_error(ValidationErrors.HED_TAGS_NOT_ALLOWED, "Red",
93+
"(Definition/InvalidDefA,Red,Blue)"),
94+
'extraOtherTags': self.format_error(ValidationErrors.HED_TAGS_NOT_ALLOWED, "Black",
95+
"(Definition/InvalidDef1,Black)"),
9496
'duplicateDef': self.format_error(DefinitionErrors.DUPLICATE_DEFINITION, "Def1"),
9597
'duplicateDef2': self.format_error(DefinitionErrors.DUPLICATE_DEFINITION, "Def1"),
9698

9799
'defTooManyPlaceholders': self.format_error(DefinitionErrors.WRONG_NUMBER_PLACEHOLDER_TAGS,
98100
"TestDefPlaceholder", expected_count=1,
99-
tag_list=["Age/#", "Item/TestDef2/#"]),
100-
'invalidPlaceholderExtension': self.format_error(DefinitionErrors.INVALID_DEFINITION_EXTENSION,
101-
tag=0, def_name="InvalidDef1/this-part-is-not-allowed"),
102-
'invalidPlaceholder': self.format_error(DefinitionErrors.INVALID_DEFINITION_EXTENSION,
103-
tag=0, def_name="InvalidDef1/InvalidPlaceholder"),
101+
tag_list=["Age/#", "Item-count/#"]),
102+
'invalidPlaceholderExtension': self.format_error(ValidationErrors.INVALID_VALUE_CLASS_CHARACTER,
103+
"Definition/InvalidDef1/this-part-is-not-allowed/#", "/", value_class="nameClass"),
104+
'invalidPlaceholder': self.format_error(ValidationErrors.INVALID_VALUE_CLASS_CHARACTER,
105+
"Definition/InvalidDef1/InvalidPlaceholder", "/", value_class="nameClass"),
104106
'defInGroup': self.format_error(DefinitionErrors.DEF_TAG_IN_DEFINITION,
105-
tag=HedTag("Def/ImproperlyPlacedDef", self.hed_schema),
107+
tag=HedTag("Def/OkayDef", self.hed_schema),
106108
def_name="ValidDefName"),
107-
'defExpandInGroup': self.format_error(DefinitionErrors.DEF_TAG_IN_DEFINITION,
108-
tag=HedTag("Def-expand/ImproperlyPlacedDef", self.hed_schema),
109-
def_name="ValidDefName"),
109+
'defExpandInGroup': self.format_error(ValidationErrors.HED_TAGS_NOT_ALLOWED,
110+
tag=HedTag("Definition/ValidDefName", self.hed_schema),
111+
group="(Definition/ValidDefName,(Def-expand/OkayDef,(White)))"),
110112
'doublePoundSignPlaceholder': self.format_error(DefinitionErrors.INVALID_DEFINITION_EXTENSION,
111113
tag=0, def_name="InvalidDef/##"),
112114
'doublePoundSignDiffPlaceholder': self.format_error(DefinitionErrors.WRONG_NUMBER_PLACEHOLDER_TAGS,
@@ -152,14 +154,16 @@ def test_add_definition(self):
152154
def_dict = DefinitionDict()
153155
def_dict.add_definitions("(Definition/testdefplaceholder,(Acceleration/#,Item/TestDef2,Red))",
154156
self.hed_schema)
155-
self.assertEqual(len(def_dict.issues), 1)
157+
self.assertEqual(len(def_dict.issues), 2)
158+
errors = [issue for issue in def_dict.issues if issue['severity'] < ErrorSeverity.WARNING]
159+
self.assertEqual(len(errors), 1)
156160
self.assertEqual(len(def_dict.defs), 0)
157161

158162
# Good input string
159163
def_dict2 = DefinitionDict()
160164
def_dict2.add_definitions("(Definition/testdefplaceholder/#,(Acceleration/#,Item/TestDef2, Red))",
161165
self.hed_schema)
162-
self.assertEqual(len(def_dict2.issues), 0)
166+
self.assertEqual(len(def_dict2.issues), 1)
163167
self.assertEqual(len(def_dict2.defs), 1)
164168

165169

‎tests/models/test_sidecar.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import io
44
import shutil
55

6-
from hed.errors import HedFileError, ValidationErrors
6+
from hed.errors import HedFileError, ValidationErrors, ErrorSeverity
77
from hed.models import ColumnMetadata, HedString, Sidecar
88
from hed import schema
99
from hed.models import DefinitionDict, DefinitionEntry
@@ -115,10 +115,14 @@ def test_duplicate_def(self):
115115
# If external defs are the same, no error
116116
duplicate_dict = sidecar.extract_definitions(hed_schema=self.hed_schema)
117117
issues = sidecar.validate(self.hed_schema, extra_def_dicts=duplicate_dict, error_handler=ErrorHandler(False))
118-
self.assertEqual(len(issues), 0)
118+
self.assertEqual(len(issues), 2)
119+
errors = [issue for issue in issues if issue['severity'] < ErrorSeverity.WARNING]
120+
self.assertEqual(len(errors), 0)
119121
test_dict = {"jsonfiledef3": DefinitionEntry("jsonfiledef3", None, False, None)}
120122
issues2 = sidecar.validate(self.hed_schema, extra_def_dicts=test_dict, error_handler=ErrorHandler(False))
121-
self.assertEqual(len(issues2), 1)
123+
self.assertEqual(len(issues2), 3)
124+
errors = [issue for issue in issues2 if issue['severity'] < ErrorSeverity.WARNING]
125+
self.assertEqual(len(errors), 1)
122126
self.assertTrue(issues2[0]['code'], ValidationErrors.DEFINITION_INVALID)
123127

124128
def test_save_load(self):

‎tests/tools/analysis/test_hed_type_factors.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,14 @@ def setUpClass(cls):
2424
HedString(
2525
'(Definition/Cond3/#, (Organizational-property/Condition-variable/Var3, Label/#, Ellipse, Cross))',
2626
hed_schema=schema),
27-
HedString('(Definition/Cond4, (Condition-variable, Apple, Banana))', hed_schema=schema),
28-
HedString('(Definition/Cond5, (Condition-variable/Lumber, Apple, Banana))', hed_schema=schema),
29-
HedString('(Definition/Cond6/#, (Condition-variable/Lumber, Label/#, Apple, Banana))',
27+
HedString('(Definition/Cond4, (Condition-variable, Item, Circle))', hed_schema=schema),
28+
HedString('(Definition/Cond5, (Condition-variable/Lumber, Item, Circle))', hed_schema=schema),
29+
HedString('(Definition/Cond6/#, (Condition-variable/Lumber, Label/#, Item, Circle))',
3030
hed_schema=schema)]
3131
def_dict = DefinitionDict()
3232
for value in defs:
3333
def_dict.check_for_definitions(value)
34+
cls.setup_dict = def_dict
3435
test_strings1 = ["Sensory-event,(Def/Cond1,(Red, Blue, Condition-variable/Trouble),Onset)",
3536
"(Def/Cond2,Onset),Green,Yellow, Def/Cond5, Def/Cond6/4",
3637
"(Def/Cond1, Offset)",

‎tests/validator/test_def_validator.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ def test_duplicate_def(self):
8888
def_dict.check_for_definitions(def_string, error_handler=error_handler)
8989
self.assertEqual(len(def_dict.issues), 0)
9090
def_dict2 = DefinitionDict()
91-
def_dict2.add_definitions("(Definition/testdefplaceholder/#,(Acceleration/#,Item/TestDef2, Red))", self.hed_schema)
91+
def_dict2.add_definitions("(Definition/testdefplaceholder/#,(Acceleration/#,Item, Red))", self.hed_schema)
9292
self.assertEqual(len(def_dict2.issues), 0)
9393
def_dict3 = DefinitionDict([def_dict, def_dict2])
9494
self.assertEqual(len(def_dict3.issues), 1)

0 commit comments

Comments
 (0)