Merge pull request #1236 from VisLab/fix_extras

Add more compliance tests for the extras

Author: VisLab

hed/errors/error_types.py

@@ -154,6 +154,14 @@ class SchemaAttributeErrors:
 
     SCHEMA_GENERIC_ATTRIBUTE_VALUE_INVALID = "SCHEMA_GENERIC_ATTRIBUTE_VALUE_INVALID"
 
+    # Extras section errors
+    SCHEMA_MISSING_EXTRA_VALUE = "SCHEMA_MISSING_EXTRA_VALUE"
+
+    # Annotation attribute errors
+    SCHEMA_ANNOTATION_PREFIX_MISSING = "SCHEMA_ANNOTATION_PREFIX_MISSING"
+    SCHEMA_ANNOTATION_EXTERNAL_MISSING = "SCHEMA_ANNOTATION_EXTERNAL_MISSING"
+    SCHEMA_ANNOTATION_SOURCE_MISSING = "SCHEMA_ANNOTATION_SOURCE_MISSING"
+
 
 class DefinitionErrors:
     # These are all DEFINITION_INVALID errors

hed/errors/schema_error_messages.py

@@ -186,3 +186,41 @@ def schema_error_SCHEMA_ALLOWED_CHARACTERS_INVALID(tag, invalid_character):
 @hed_error(SchemaAttributeErrors.SCHEMA_IN_LIBRARY_INVALID, actual_code=SchemaAttributeErrors.SCHEMA_ATTRIBUTE_VALUE_INVALID)
 def schema_error_SCHEMA_IN_LIBRARY_INVALID(tag, bad_library):
     return f"Tag '{tag}' has an invalid inLibrary: '{bad_library}'.  "
+
+
+@hed_error(SchemaAttributeErrors.SCHEMA_MISSING_EXTRA_VALUE, default_severity=ErrorSeverity.WARNING)
+def schema_error_SCHEMA_MISSING_EXTRA_VALUE(section_name, column_name, row_index):
+    return f"Extras section '{section_name}' has an empty value in column '{column_name}' " f"at row {row_index}."
+
+
+@hed_error(
+    SchemaAttributeErrors.SCHEMA_ANNOTATION_PREFIX_MISSING,
+    default_severity=ErrorSeverity.WARNING,
+)
+def schema_error_annotation_prefix_missing(tag, annotation_value, prefix):
+    return (
+        f"Tag '{tag}' has annotation '{annotation_value}' with prefix '{prefix}' "
+        f"that is not defined in the Prefixes section."
+    )
+
+
+@hed_error(
+    SchemaAttributeErrors.SCHEMA_ANNOTATION_EXTERNAL_MISSING,
+    default_severity=ErrorSeverity.WARNING,
+)
+def schema_error_annotation_external_missing(tag, annotation_value, prefix, annotation_id):
+    return (
+        f"Tag '{tag}' has annotation '{annotation_value}' with prefix:id '{prefix}{annotation_id}' "
+        f"that is not defined in the ExternalAnnotations section."
+    )
+
+
+@hed_error(
+    SchemaAttributeErrors.SCHEMA_ANNOTATION_SOURCE_MISSING,
+    default_severity=ErrorSeverity.WARNING,
+)
+def schema_error_annotation_source_missing(tag, annotation_value, source_text):
+    return (
+        f"Tag '{tag}' has annotation '{annotation_value}' referencing dc:source "
+        f"but '{source_text}' does not start with any name defined in the Sources section."
+    )

hed/schema/schema_validation/compliance.py

@@ -15,6 +15,7 @@
 
 from functools import partial
 
+import pandas as pd
 from semantic_version import Version
 
 from hed.errors.error_reporter import ErrorHandler, sort_issues
@@ -28,6 +29,7 @@
 from hed.schema import hed_cache
 from hed.schema.schema_validation import attribute_validators
 from hed.schema.hed_schema import HedSchema, HedKey, HedSectionKey
+from hed.schema.schema_io import df_constants
 from hed.schema.schema_validation.hed_id_validator import HedIDValidator
 from hed.schema.schema_validation.compliance_summary import ComplianceSummary
 from hed.schema.schema_validation.validation_util import (
@@ -77,6 +79,8 @@ def check_compliance(hed_schema, check_for_warnings=True, name=None, error_handl
     issues += validator.check_invalid_characters()
     issues += validator.check_attributes()
     issues += validator.check_duplicate_names()
+    issues += validator.check_extras_columns()
+    issues += validator.check_annotation_attribute_values()
 
     error_handler.pop_error_context()
     issues = sort_issues(issues)
@@ -328,6 +332,218 @@ def check_duplicate_names(self):
         self.summary.record_issues(len(issues))
         return issues
 
+    def check_extras_columns(self):
+        """Validate that all extras DataFrames have non-empty values in required columns.
+
+        For each extras section (Sources, Prefixes, ExternalAnnotations), checks
+        that every cell in the required columns defined in
+        ``df_constants.extras_column_dict`` has a non-empty value.
+
+        Note:
+            Missing columns are automatically added with empty strings during
+            schema loading (see ``base2schema.fix_extra``), so only value
+            presence needs to be checked here.
+        """
+        self.summary.start_check(
+            "extras_columns",
+            "Validate extras sections have non-empty values in required columns.",
+        )
+        self.summary.add_sub_check("non-empty cell values")
+
+        issues = []
+        extras = getattr(self.hed_schema, "extras", {}) or {}
+        for section_name, required_cols in df_constants.extras_column_dict.items():
+            df = extras.get(section_name)
+            if df is None or (isinstance(df, pd.DataFrame) and df.empty):
+                # Empty extras are fine — nothing to validate
+                continue
+
+            rows_checked = len(df)
+            self.summary.record_section(section_name, rows_checked)
+
+            for col in required_cols:
+                if col not in df.columns:
+                    continue
+                mask = df[col].isna() | df[col].astype(str).str.strip().eq("")
+                for row_idx in mask[mask].index:
+                    issues += ErrorHandler.format_error(
+                        SchemaAttributeErrors.SCHEMA_MISSING_EXTRA_VALUE,
+                        section_name=section_name,
+                        column_name=col,
+                        row_index=row_idx,
+                    )
+
+        self.error_handler.add_context_and_filter(issues)
+        self.summary.record_issues(len(issues))
+        return issues
+
+    def check_annotation_attribute_values(self):
+        """Validate that annotation attribute values reference valid prefixes, external annotations, and sources.
+
+        For each entry that has an ``annotation`` attribute, checks that:
+
+        1. The value starts with ``prefix:id`` where ``prefix:`` is defined in
+           the Prefixes extras section and ``prefix:`` + ``id`` is a row in the
+           ExternalAnnotations extras section.
+        2. If the annotation references ``dc:source``, the remaining text after
+           ``dc:source `` must start with a name from the Sources extras section.
+        """
+        self.summary.start_check(
+            "annotation_attributes",
+            "Validate annotation attribute values reference defined prefixes, external annotations, and sources.",
+        )
+        self.summary.add_sub_check("prefix defined in Prefixes")
+        self.summary.add_sub_check("prefix:id in ExternalAnnotations")
+        self.summary.add_sub_check("dc:source references valid Sources entry")
+
+        issues = []
+
+        # Build lookup sets from extras
+        extras = getattr(self.hed_schema, "extras", {}) or {}
+        defined_prefixes = self._get_extras_column_values(extras, df_constants.PREFIXES_KEY, df_constants.prefix)
+        external_pairs = self._get_external_annotation_pairs(extras)
+        defined_sources = self._get_extras_column_values(extras, df_constants.SOURCES_KEY, df_constants.source)
+
+        # Scan all entries in all sections for the "annotation" attribute
+        entries_checked = 0
+        for section_key in HedSectionKey:
+            self.error_handler.push_error_context(ErrorContext.SCHEMA_SECTION, str(section_key))
+            for entry in self.hed_schema[section_key].values():
+                annotation_value = entry.attributes.get("annotation")
+                if not annotation_value:
+                    continue
+                entries_checked += 1
+                self.error_handler.push_error_context(ErrorContext.SCHEMA_TAG, entry.name)
+                # Annotation values can be comma-separated (multiple annotations)
+                for single_annotation in annotation_value.split(","):
+                    single_annotation = single_annotation.strip()
+                    if single_annotation:
+                        issues += self._validate_annotation_value(
+                            entry, single_annotation, defined_prefixes, external_pairs, defined_sources
+                        )
+                self.error_handler.pop_error_context()
+            self.error_handler.pop_error_context()
+
+        self.summary.record_section("annotation_entries", entries_checked)
+        self.summary.record_issues(len(issues))
+        return issues
+
+    # -----------------------------------------------------------------------
+    # Private helpers — extras / annotation validation
+    # -----------------------------------------------------------------------
+
+    @staticmethod
+    def _get_extras_column_values(extras, section_key, column_name):
+        """Return the set of values in a column of an extras DataFrame.
+
+        Parameters:
+            extras (dict): The schema extras dictionary.
+            section_key (str): Key into the extras dict (e.g. "Prefixes").
+            column_name (str): The column whose values to collect.
+
+        Returns:
+            set: The set of non-empty string values in that column.
+        """
+        df = extras.get(section_key)
+        if df is None or not isinstance(df, pd.DataFrame) or df.empty:
+            return set()
+        if column_name not in df.columns:
+            return set()
+        return {str(v).strip() for v in df[column_name] if pd.notna(v) and str(v).strip()}
+
+    @staticmethod
+    def _get_external_annotation_pairs(extras):
+        """Return a set of (prefix, id) tuples from the ExternalAnnotations DataFrame.
+
+        Parameters:
+            extras (dict): The schema extras dictionary.
+
+        Returns:
+            set: Set of (prefix_str, id_str) tuples.
+        """
+        df = extras.get(df_constants.EXTERNAL_ANNOTATION_KEY)
+        if df is None or not isinstance(df, pd.DataFrame) or df.empty:
+            return set()
+        pairs = set()
+        if df_constants.prefix in df.columns and df_constants.id in df.columns:
+            for _, row in df.iterrows():
+                p = str(row[df_constants.prefix]).strip() if pd.notna(row[df_constants.prefix]) else ""
+                i = str(row[df_constants.id]).strip() if pd.notna(row[df_constants.id]) else ""
+                if p and i:
+                    pairs.add((p, i))
+        return pairs
+
+    def _validate_annotation_value(self, entry, annotation_value, defined_prefixes, external_pairs, defined_sources):
+        """Validate a single annotation attribute value.
+
+        Parameters:
+            entry: The schema entry with the annotation attribute.
+            annotation_value (str): The annotation value string.
+            defined_prefixes (set): Valid prefixes from the Prefixes section.
+            external_pairs (set): Valid (prefix, id) pairs from ExternalAnnotations.
+            defined_sources (set): Valid source names from the Sources section.
+
+        Returns:
+            list: A list of issue dicts.
+        """
+        issues = []
+        tag_name = entry.name
+
+        # Parse prefix:id from the annotation value
+        # Expected format: "prefix:id rest_of_text" e.g. "dc:source Beniczky ea 2017 Table 2."
+        colon_pos = annotation_value.find(":")
+        if colon_pos < 1:
+            # No colon found — cannot parse prefix:id
+            issues += self.error_handler.format_error_with_context(
+                SchemaAttributeErrors.SCHEMA_ANNOTATION_PREFIX_MISSING,
+                tag_name,
+                annotation_value=annotation_value,
+                prefix="(none)",
+            )
+            return issues
+
+        ann_prefix = annotation_value[: colon_pos + 1]  # e.g. "dc:"
+        remainder = annotation_value[colon_pos + 1 :]  # e.g. "source Beniczky ea 2017 Table 2."
+
+        # Split remainder into id and rest — id is the first whitespace-delimited token
+        parts = remainder.split(None, 1)  # split on whitespace, max 1 split
+        ann_id = parts[0] if parts else remainder  # e.g. "source"
+        rest_text = parts[1] if len(parts) > 1 else ""  # e.g. "Beniczky ea 2017 Table 2."
+
+        # Check 1: prefix must be in Prefixes
+        if ann_prefix not in defined_prefixes:
+            issues += self.error_handler.format_error_with_context(
+                SchemaAttributeErrors.SCHEMA_ANNOTATION_PREFIX_MISSING,
+                tag_name,
+                annotation_value=annotation_value,
+                prefix=ann_prefix,
+            )
+
+        # Check 2: prefix:id must be in ExternalAnnotations
+        if (ann_prefix, ann_id) not in external_pairs:
+            issues += self.error_handler.format_error_with_context(
+                SchemaAttributeErrors.SCHEMA_ANNOTATION_EXTERNAL_MISSING,
+                tag_name,
+                annotation_value=annotation_value,
+                prefix=ann_prefix,
+                annotation_id=ann_id,
+            )
+
+        # Check 3: If dc:source, the rest_text must start with a defined source name
+        if ann_prefix == "dc:" and ann_id == "source":
+            rest_text_stripped = rest_text.strip() if rest_text else ""
+            if not rest_text_stripped or not any(rest_text_stripped.startswith(src) for src in defined_sources):
+                issues += self.error_handler.format_error_with_context(
+                    SchemaAttributeErrors.SCHEMA_ANNOTATION_SOURCE_MISSING,
+                    tag_name,
+                    annotation_value=annotation_value,
+                    source_text=rest_text_stripped,
+                )
+
+        for issue in issues:
+            issue["severity"] = ErrorSeverity.WARNING
+        return issues
+
     # -----------------------------------------------------------------------
     # Private helpers — attribute validation
     # -----------------------------------------------------------------------
@@ -388,8 +604,6 @@ def _run_validators(self, entry, attribute_name, validators):
         for validator in validators:
             self.error_handler.push_error_context(ErrorContext.SCHEMA_ATTRIBUTE, attribute_name)
             new_issues = validator(self.hed_schema, entry, attribute_name)
-            for issue in new_issues:
-                issue["severity"] = ErrorSeverity.WARNING
             self.error_handler.add_context_and_filter(new_issues)
             issues += new_issues
             self.error_handler.pop_error_context()

hed/scripts/schema_script_util.py

@@ -1,7 +1,8 @@
 import os.path
 from collections import defaultdict
 from hed.schema import from_string, load_schema, from_dataframes
-from hed.errors import get_printable_issue_string, HedFileError, SchemaWarnings
+from hed.errors import get_printable_issue_string, HedFileError
+from hed.errors.error_types import ErrorSeverity
 from hed.schema.schema_comparer import SchemaComparer
 
 all_extensions = [".tsv", ".mediawiki", ".xml", ".json"]
@@ -23,7 +24,7 @@ def validate_schema_object(base_schema, schema_name):
     validation_issues = []
     try:
         issues = base_schema.check_compliance()
-        issues = [issue for issue in issues if issue["code"] != SchemaWarnings.SCHEMA_PRERELEASE_VERSION_USED]
+        issues = [issue for issue in issues if issue.get("severity", ErrorSeverity.ERROR) == ErrorSeverity.ERROR]
         if issues:
             error_message = get_printable_issue_string(issues, title=schema_name)
             validation_issues.append(error_message)

spec_tests/test_errors.py

@@ -442,6 +442,14 @@ def _run_single_schema_test(self, info, error_code, all_codes, description, name
                 try:
                     loaded_schema = from_string(schema_string, schema_format=".mediawiki")
                     issues = loaded_schema.check_compliance()
+                    # Filter annotation compliance warnings — these are noise from schemas
+                    # lacking complete ExternalAnnotations/Prefixes/Sources sections
+                    _ANNOTATION_CODES = {
+                        "SCHEMA_ANNOTATION_PREFIX_MISSING",
+                        "SCHEMA_ANNOTATION_EXTERNAL_MISSING",
+                        "SCHEMA_ANNOTATION_SOURCE_MISSING",
+                    }
+                    issues = [i for i in issues if i.get("code") not in _ANNOTATION_CODES]
                 except HedFileError as e:
                     issues = e.issues
                     if not issues: