Fixing assembly rules

hed/validator/hed_validator.py

@@ -94,11 +94,14 @@ def run_basic_checks(self, hed_string, allow_placeholders) -> list[dict]:
         issues += self._def_validator.validate_def_tags(hed_string)
         return issues
 
-    def run_full_string_checks(self, hed_string) -> list[dict]:
+    def run_full_string_checks(self, hed_string, whole_row=True) -> list[dict]:
         """Run all full-string validation checks on a HED string.
 
         Parameters:
             hed_string (HedString): The HED string to validate.
+            whole_row (bool): If False, the string is one cell of a row whose other columns also carry HED,
+                so the checks that need every tag of the row (required and unique tags) are skipped. The
+                group-level checks still apply: a group in the cell is a group of the row.
 
         Returns:
             list[dict]: A list of issues found during validation. Each issue is represented as a dictionary.
@@ -111,10 +114,11 @@ def run_full_string_checks(self, hed_string) -> list[dict]:
 
         """
         checks = [
-            self._group_validator.run_all_tags_validators,
             self._group_validator.run_tag_level_validators,
             self._def_validator.validate_onset_offset,
         ]
+        if whole_row:
+            checks.insert(0, self._group_validator.run_all_tags_validators)
 
         for check in checks:
             issues = check(hed_string)  # Call each function with `hed_string`

hed/validator/spreadsheet_validator.py

@@ -40,8 +40,10 @@ class SpreadsheetValidator:
     2. **Column values**: the column structure (mapper issues, sidecar references to columns the table
        lacks), then each value column's distinct values against the units or value class of its ``#``
        tag, and each categorical column's distinct values against its sidecar keys.
-    3. **HED column**: each distinct string of every HED-tags column, the basic (single-string) checks;
-       for a source that offers no assembly, the group-level checks too, since no stage 4 follows.
+    3. **HED column**: each distinct string of every HED-tags column, the basic (single-string) checks.
+       For a source that offers no assembly, the group-level checks follow on the same strings, since no
+       stage 4 will run them on the whole row. The required-tag and unique-tag checks belong to the
+       assembled row and never run on a cell.
     4. **Assembly**: the rows assembled from all columns, the group-level checks, and the onset and
        temporal checks of a timeline file.
 
@@ -215,24 +217,27 @@ def _validate_source(self, source, sidecar, extra_def_dicts, error_handler, row_
         if check_for_any_errors(issues):
             return sort_issues(issues)
 
-        # Stage 3: every HED-tags column, one parse per distinct string. A source that will never
-        # assemble rows gets the full-string checks here, since no stage 4 will run them on the whole row.
-        base_input = source.as_base_input()
-        for column in columns:
-            if column.column_type == ColumnType.HEDTags:
-                issues += self.validate_hed_column(
-                    column.column_name,
-                    source.distinct_values(column.column_name),
-                    error_handler,
-                    row_offset=row_offset,
-                    full_string=base_input is None,
-                )
+        # Stage 3: every HED-tags column, one parse per distinct string.
+        hed_columns = [column for column in columns if column.column_type == ColumnType.HEDTags]
+        for column in hed_columns:
+            issues += self.validate_hed_column(
+                column.column_name, source.distinct_values(column.column_name), error_handler, row_offset=row_offset
+            )
         if check_for_any_errors(issues):
             return sort_issues(issues)
 
-        # Stage 4: the assembled rows, only for a source that can provide them.
+        # Stage 4: the assembled rows, only for a source that can provide them. The source is asked only
+        # now, so a stage 3 error never costs a table conversion. A source that offers no assembly gets
+        # the group-level checks on each distinct HED cell instead, since no stage 4 will run them on the
+        # whole row; the required-tag and unique-tag checks stay with the assembled row.
+        base_input = source.as_base_input()
         if base_input is not None:
             issues += self.validate_assembled(base_input, error_handler, row_offset=row_offset)
+            return sort_issues(issues)
+        for column in hed_columns:
+            issues += self._validate_hed_cells(
+                column.column_name, source.distinct_values(column.column_name), error_handler, row_offset=row_offset
+            )
         return sort_issues(issues)
 
     @staticmethod
@@ -376,9 +381,10 @@ def validate_hed_column(self, column_name, distinct, error_handler, row_offset=0
             distinct (dict[str, list[int]]): Distinct cell text -> 0-based rows holding it.
             error_handler (ErrorHandler): Holds context.
             row_offset (int): Added to the first row of each reported string.
-            full_string (bool): If True, also run the full-string (group-level) checks on each string
-                that passes the basic checks. For a caller that will never assemble rows; otherwise
-                those checks belong to the assembly stage, where the row is whole.
+            full_string (bool): If True, also run the group-level checks on each string that passes the
+                basic checks. For a caller that will never assemble rows; otherwise those checks belong
+                to the assembly stage, where the row is whole. The required-tag and unique-tag checks
+                need the assembled row and never run here.
 
         Returns:
             list[dict]: The issues found, each with ``row_count``.
@@ -388,21 +394,37 @@ def validate_hed_column(self, column_name, distinct, error_handler, row_offset=0
             hed_string = HedString(text, self._schema)
             string_issues = self._hed_validator.run_basic_checks(hed_string, allow_placeholders=False)
             if full_string and not check_for_any_errors(string_issues):
-                string_issues += self._hed_validator.run_full_string_checks(hed_string)
-            if not string_issues:
-                continue
-            error_handler.push_error_context(ErrorContext.ROW, rows[0] + row_offset)
-            error_handler.push_error_context(ErrorContext.COLUMN, column_name)
-            error_handler.push_error_context(ErrorContext.HED_STRING, hed_string)
-            error_handler.add_context_and_filter(string_issues)
-            error_handler.pop_error_context()  # HedString
-            error_handler.pop_error_context()  # Column
-            error_handler.pop_error_context()  # Row
-            for issue in string_issues:
-                issue[ROW_COUNT_KEY] = len(rows)
-            issues += string_issues
+                string_issues += self._hed_validator.run_full_string_checks(hed_string, whole_row=False)
+            issues += self._report_distinct(string_issues, hed_string, rows, column_name, error_handler, row_offset)
+        return issues
+
+    def _validate_hed_cells(self, column_name, distinct, error_handler, row_offset) -> list[dict]:
+        """Run the group-level checks on each distinct string of a HED-tags column that has passed stage 3,
+        for a source that offers no assembly. One more parse per distinct string. A cell is part of a row,
+        so the required-tag and unique-tag checks are left to assembly."""
+        issues = []
+        for text, rows in distinct.items():
+            hed_string = HedString(text, self._schema)
+            string_issues = self._hed_validator.run_full_string_checks(hed_string, whole_row=False)
+            issues += self._report_distinct(string_issues, hed_string, rows, column_name, error_handler, row_offset)
         return issues
 
+    @staticmethod
+    def _report_distinct(string_issues, hed_string, rows, column_name, error_handler, row_offset) -> list[dict]:
+        """Give the issues of one distinct string their row, column, and string context and ``row_count``."""
+        if not string_issues:
+            return []
+        error_handler.push_error_context(ErrorContext.ROW, rows[0] + row_offset)
+        error_handler.push_error_context(ErrorContext.COLUMN, column_name)
+        error_handler.push_error_context(ErrorContext.HED_STRING, hed_string)
+        error_handler.add_context_and_filter(string_issues)
+        error_handler.pop_error_context()  # HedString
+        error_handler.pop_error_context()  # Column
+        error_handler.pop_error_context()  # Row
+        for issue in string_issues:
+            issue[ROW_COUNT_KEY] = len(rows)
+        return string_issues
+
     # ------------------------------------------------------------------------------------------
     # Stage 4
 

tests/validator/test_staged_validation.py

@@ -2,11 +2,12 @@
 
 import io
 import json
+import os
 import unittest
 
 import pandas as pd
 
-from hed import Sidecar, SpreadsheetInput, TabularInput, load_schema_version
+from hed import Sidecar, SpreadsheetInput, TabularInput, load_schema, load_schema_version
 from hed.errors.error_reporter import ErrorHandler
 from hed.errors.error_types import ErrorContext, SidecarErrors, ValidationErrors
 from hed.models import DefinitionDict, ListColumnSource
@@ -23,6 +24,18 @@ def _tabular(rows, sidecar=None, name=None):
     return TabularInput(io.BytesIO(text.encode("utf-8")), sidecar=sidecar, name=name)
 
 
+class _CountingSource(ListColumnSource):
+    """A ListColumnSource that records how often the validator asks for its BaseInput."""
+
+    def __init__(self, columns, base_input=None):
+        super().__init__(columns, base_input=base_input)
+        self.base_input_calls = 0
+
+    def as_base_input(self):
+        self.base_input_calls += 1
+        return super().as_base_input()
+
+
 class TestStagedValidation(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
@@ -225,21 +238,72 @@ def test_column_source_with_a_base_input_reaches_assembly(self):
         self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.TAG_EXPRESSION_REPEATED])
         self.assertEqual((issues[0][ErrorContext.ROW], issues[0][ROW_COUNT_KEY]), (1, 1))
 
-    def test_plain_source_gets_the_full_string_checks_in_the_hed_column_stage(self):
-        """A source with no BaseInput never reaches assembly, so stage 3 runs the group-level checks on each
-        distinct string; the same columns with a BaseInput leave those checks to stage 4, once."""
+    def test_plain_source_gets_the_full_string_checks_after_the_hed_column_stage(self):
+        """A source with no BaseInput never reaches assembly, so the group-level checks run on each distinct
+        HED string once stage 3 has passed; the same columns with a BaseInput leave them to stage 4, once."""
         columns = {"onset": [1.0, 2.0, 3.0], "HED": ["Red", "(Onset)", "(Onset)"]}
 
-        issues = self.validator.validate(ListColumnSource(columns))
+        source = _CountingSource(columns)
+        issues = self.validator.validate(source)
         self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.TEMPORAL_TAG_ERROR])
         self.assertEqual((issues[0][ErrorContext.ROW], issues[0][ROW_COUNT_KEY]), (1, 2))
+        self.assertEqual(source.base_input_calls, 1)
 
         frame = TabularInput(pd.DataFrame({name: [str(v) for v in values] for name, values in columns.items()}))
         issues = self.validator.validate(ListColumnSource(columns, base_input=frame))
         self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.TEMPORAL_TAG_ERROR] * 2)
         self.assertEqual([issue[ErrorContext.ROW] for issue in issues], [1, 2])
         self.assertTrue(all(ROW_COUNT_KEY not in issue for issue in issues))
 
+    def test_base_input_is_not_asked_for_when_an_earlier_stage_stops(self):
+        """Building a BaseInput may mean converting the whole table, so the validator asks for it only once
+        stages 2 and 3 have passed; a stage 3 error is reported without that cost."""
+        source = _CountingSource({"onset": [1.0, 2.0], "HED": ["Red", "InvalidTagXYZ"]})
+        issues = self.validator.validate(source)
+        self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.TAG_INVALID])
+        self.assertEqual(source.base_input_calls, 0)
+
+        source = _CountingSource({"onset": [1.0], "response_time": ["abc"], "HED": ["Red"]})
+        issues = self.validator.validate(source, sidecar=_sidecar(self.sidecar_dict), extra_def_dicts=self.definitions)
+        self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.VALUE_INVALID])
+        self.assertEqual(source.base_input_calls, 0)
+
+    def test_required_and_unique_tags_are_checked_only_on_assembled_rows(self):
+        """A HED cell is part of a row, so the required-tag and unique-tag checks never run on it alone: the
+        sidecar may supply the tags. They run in the assembly stage; the group-level checks apply to a cell."""
+        schema_path = os.path.join(
+            os.path.dirname(os.path.realpath(__file__)), "../data/validator_tests/HED8.0.0_added_tests.mediawiki"
+        )
+        validator = SpreadsheetValidator(load_schema(schema_path))
+        sidecar = _sidecar({"condition": {"HED": {"a": "Action, Animal-agent"}}})
+
+        # The row assembles to "Action, Animal-agent, Event" and satisfies the required tags.
+        source = ListColumnSource({"condition": ["a"], "HED": ["Event"]})
+        self.assertEqual(validator.validate(source, sidecar=sidecar), [])
+
+        # A group-level error in the cell is still an error of the row.
+        source = ListColumnSource({"condition": ["a"], "HED": ["Event, (Onset)"]})
+        issues = validator.validate(source, sidecar=sidecar)
+        self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.TEMPORAL_TAG_ERROR])
+
+        # Without assembly a HED-only table is not checked for required tags either; with assembly it is.
+        columns = {"onset": [1.0], "duration": [0], "HED": ["Event"]}
+        self.assertEqual(validator.validate(ListColumnSource(columns)), [])
+        frame = TabularInput(pd.DataFrame({name: [str(v) for v in values] for name, values in columns.items()}))
+        issues = validator.validate(ListColumnSource(columns, base_input=frame))
+        self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.REQUIRED_TAG_MISSING] * 2)
+
+        # The same holds for a unique tag repeated within one cell.
+        columns = {
+            "onset": [1.0],
+            "duration": [0],
+            "HED": ["(Event-context, (Red, Blue)), (Event-context, (Green, Yellow))"],
+        }
+        self.assertEqual(self.validator.validate(ListColumnSource(columns)), [])
+        frame = TabularInput(pd.DataFrame({name: [str(v) for v in values] for name, values in columns.items()}))
+        issues = self.validator.validate(ListColumnSource(columns, base_input=frame))
+        self.assertEqual([issue["code"] for issue in issues], [ValidationErrors.TAG_NOT_UNIQUE])
+
     def test_plain_source_gets_a_warning_for_a_column_the_sidecar_does_not_describe(self):
         source = ListColumnSource({"id": [1, 2], "onset": [1.0, 2.0], "HED": ["Red", "Blue"]})
         issues = self.validator.validate(