Skip to content

Commit 8554894

Browse files
committed
Sidecar strings skip the required-tag check
Copilot review of PR hed-standard#1437. A sidecar string is a fragment of a row, so the sidecar stage no longer runs the required-tag check on it; required tags split between a sidecar string and the HED column validate clean. The unique-tag check still runs there, since a unique tag repeated inside a fragment is repeated in the row and hed-tests has a sidecar-only TAG_NOT_UNIQUE case. run_full_string_checks takes required_tags and unique_tags in place of whole_row; the HED-cell pass passes both False.
1 parent fe365d5 commit 8554894

5 files changed

Lines changed: 70 additions & 10 deletions

File tree

‎hed/validator/hed_validator.py‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,16 @@ def run_basic_checks(self, hed_string, allow_placeholders) -> list[dict]:
9494
issues += self._def_validator.validate_def_tags(hed_string)
9595
return issues
9696

97-
def run_full_string_checks(self, hed_string, whole_row=True) -> list[dict]:
97+
def run_full_string_checks(self, hed_string, required_tags=True, unique_tags=True) -> list[dict]:
9898
"""Run all full-string validation checks on a HED string.
9999
100100
Parameters:
101101
hed_string (HedString): The HED string to validate.
102-
whole_row (bool): If False, the string is one cell of a row whose other columns also carry HED,
103-
so the checks that need every tag of the row (required and unique tags) are skipped. The
104-
group-level checks still apply: a group in the cell is a group of the row.
102+
required_tags (bool): Run the required-tag check. Pass False for a fragment of a row (a HED cell,
103+
a sidecar string): the tag it lacks may come from another column, so the check needs the
104+
assembled row.
105+
unique_tags (bool): Run the unique-tag check. A unique tag repeated inside a fragment is repeated
106+
in the row too, so this check is safe on a fragment; pass False to leave it to assembly.
105107
106108
Returns:
107109
list[dict]: A list of issues found during validation. Each issue is represented as a dictionary.
@@ -111,14 +113,25 @@ def run_full_string_checks(self, hed_string, whole_row=True) -> list[dict]:
111113
- Each check is a callable function that takes `hed_string` as input and returns a list of issues.
112114
- If any check returns issues, the method stops and returns those issues immediately.
113115
- If no issues are found, an empty list is returned.
116+
- The group-level checks (tag placement, reserved groups, duplicates, Onset and Offset) always
117+
run: a group in a fragment is a group of the row.
114118
115119
"""
120+
121+
def row_tag_checks(string_obj):
122+
tags = string_obj.get_all_tags()
123+
issues = []
124+
if required_tags:
125+
issues += self._group_validator.check_for_required_tags(tags)
126+
if unique_tags:
127+
issues += self._group_validator.check_multiple_unique_tags_exist(tags)
128+
return issues
129+
116130
checks = [
131+
row_tag_checks,
117132
self._group_validator.run_tag_level_validators,
118133
self._def_validator.validate_onset_offset,
119134
]
120-
if whole_row:
121-
checks.insert(0, self._group_validator.run_all_tags_validators)
122135

123136
for check in checks:
124137
issues = check(hed_string) # Call each function with `hed_string`

‎hed/validator/sidecar_validator.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,11 @@ def _validate_sidecar(self, sidecar, extra_def_dicts, error_handler) -> list[dic
123123
modified_string = df_util.replace_ref(modified_string, f"{{{ref}}}", ref_dict[ref])
124124
hed_string_obj = HedString(modified_string, hed_schema=self._schema, def_dict=sidecar_def_dict)
125125

126+
# A sidecar string is a fragment of a row: a tag it lacks may come from another
127+
# column, so the required-tag check waits for assembly. A unique tag repeated in
128+
# the fragment is repeated in the row, so that check runs here.
126129
error_handler.push_error_context(ErrorContext.HED_STRING, hed_string_obj)
127-
new_issues += hed_validator.run_full_string_checks(hed_string_obj)
130+
new_issues += hed_validator.run_full_string_checks(hed_string_obj, required_tags=False)
128131
error_handler.add_context_and_filter(new_issues)
129132
issues += new_issues
130133
error_handler.pop_error_context() # Hed string

‎hed/validator/spreadsheet_validator.py‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ class SpreadsheetValidator:
3636
3737
The stages run in order and each stops the run when it finds an error (never on a warning alone):
3838
39-
1. **Sidecar**: every template and categorical string of the sidecar (``SidecarValidator``).
39+
1. **Sidecar**: every template and categorical string of the sidecar (``SidecarValidator``). A sidecar
40+
string is a fragment of a row, so the required-tag check waits for assembly.
4041
2. **Column values**: the column structure (mapper issues, sidecar references to columns the table
4142
lacks), then each value column's distinct values against the units or value class of its ``#``
4243
tag, and each categorical column's distinct values against its sidecar keys.
@@ -394,7 +395,9 @@ def validate_hed_column(self, column_name, distinct, error_handler, row_offset=0
394395
hed_string = HedString(text, self._schema)
395396
string_issues = self._hed_validator.run_basic_checks(hed_string, allow_placeholders=False)
396397
if full_string and not check_for_any_errors(string_issues):
397-
string_issues += self._hed_validator.run_full_string_checks(hed_string, whole_row=False)
398+
string_issues += self._hed_validator.run_full_string_checks(
399+
hed_string, required_tags=False, unique_tags=False
400+
)
398401
issues += self._report_distinct(string_issues, hed_string, rows, column_name, error_handler, row_offset)
399402
return issues
400403

@@ -405,7 +408,9 @@ def _validate_hed_cells(self, column_name, distinct, error_handler, row_offset)
405408
issues = []
406409
for text, rows in distinct.items():
407410
hed_string = HedString(text, self._schema)
408-
string_issues = self._hed_validator.run_full_string_checks(hed_string, whole_row=False)
411+
string_issues = self._hed_validator.run_full_string_checks(
412+
hed_string, required_tags=False, unique_tags=False
413+
)
409414
issues += self._report_distinct(string_issues, hed_string, rows, column_name, error_handler, row_offset)
410415
return issues
411416

‎tests/validator/test_hed_validator.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,28 @@ def test_duplicate_group_in_definition(self):
229229
issues = test_string.validate(hed_schema)
230230
self.assertEqual(len(issues), 1)
231231

232+
def test_run_full_string_checks_row_tag_flags(self):
233+
"""The required-tag and unique-tag checks need the whole row; each can be left out for a fragment,
234+
while the group-level checks always run."""
235+
schema_path = os.path.join(self.hed_base_dir, "HED8.0.0_added_tests.mediawiki")
236+
validator = HedValidator(schema.load_schema(schema_path))
237+
238+
missing_required = HedString("Event, (Onset)", validator._hed_schema)
239+
codes = [issue["code"] for issue in validator.run_full_string_checks(missing_required)]
240+
self.assertEqual(codes, ["REQUIRED_TAG_MISSING", "REQUIRED_TAG_MISSING"])
241+
codes = [issue["code"] for issue in validator.run_full_string_checks(missing_required, required_tags=False)]
242+
self.assertEqual(codes, ["TEMPORAL_TAG_ERROR"])
243+
244+
repeated_unique = HedString(
245+
"Action, Animal-agent, (Event-context, (Red, Blue)), (Event-context, (Green, Yellow))",
246+
validator._hed_schema,
247+
)
248+
codes = [issue["code"] for issue in validator.run_full_string_checks(repeated_unique)]
249+
self.assertEqual(codes, ["TAG_NOT_UNIQUE"])
250+
self.assertEqual(validator.run_full_string_checks(repeated_unique, unique_tags=False), [])
251+
codes = [issue["code"] for issue in validator.run_full_string_checks(repeated_unique, required_tags=False)]
252+
self.assertEqual(codes, ["TAG_NOT_UNIQUE"])
253+
232254
def test_severity_enum_not_string_github_hedit_161(self):
233255
"""Regression test for critical bug: severity should be ErrorSeverity enum, not string.
234256

‎tests/validator/test_staged_validation.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,23 @@ def test_required_and_unique_tags_are_checked_only_on_assembled_rows(self):
293293
issues = validator.validate(ListColumnSource(columns, base_input=frame))
294294
self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.REQUIRED_TAG_MISSING] * 2)
295295

296+
# Required tags split between a sidecar string and the HED cell: clean through a real assembled input,
297+
# so stage 1 must not hold the sidecar string alone to the required-tag rule either.
298+
split = _sidecar({"condition": {"HED": {"a": "Action"}}})
299+
frame = _tabular([["onset", "duration", "condition", "HED"], [1.0, 0, "a", "Animal-agent"]], sidecar=split)
300+
self.assertEqual(validator.validate(frame), [])
301+
self.assertEqual(
302+
validator.validate(ListColumnSource({"condition": ["a"], "HED": ["Animal-agent"]}), sidecar=split), []
303+
)
304+
305+
# A unique tag repeated inside one sidecar string is repeated in every row that uses it, so stage 1
306+
# still reports it (hed-tests TAG_NOT_UNIQUE has a sidecar-only case).
307+
repeated = _sidecar(
308+
{"condition": {"HED": {"a": "(Event-context, (Red, Blue)), (Event-context, (Green, Yellow))"}}}
309+
)
310+
issues = self.validator.validate(ListColumnSource({"condition": ["a"]}), sidecar=repeated)
311+
self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.TAG_NOT_UNIQUE])
312+
296313
# The same holds for a unique tag repeated within one cell.
297314
columns = {
298315
"onset": [1.0],

0 commit comments

Comments
 (0)