Changed schema error vs warning strategy

Author: VisLab

.github/workflows/ci_cov.yaml

@@ -37,12 +37,7 @@ jobs:
 
       # Install dependencies
       - name: Install dependencies
-        run: uv pip install ruff -e ".[docs,test]"
-
-      # Run ruff
-      - name: Lint with ruff
-        run: |
-          ruff check . --output-format=github 
+        run: uv pip install -e ".[docs,test]"
 
       # Run unittest with coverage
       - name: Test with unittest and coverage

.github/workflows/ci_windows.yaml

@@ -25,6 +25,7 @@ jobs:
         with:
           python-version: ${{ matrix.python-version }}
           enable-cache: true
+          cache-dependency-glob: "**/pyproject.toml"
 
       - name: Create virtual environment
         shell: pwsh

hed/errors/error_reporter.py

@@ -288,6 +288,22 @@ def filter_issues_by_severity(issues_list: list[dict], severity: int) -> list[di
         """
         return [issue for issue in issues_list if issue["severity"] <= severity]
 
+    @staticmethod
+    def separate_issues(issues_list: list[dict]) -> tuple[list[dict], list[dict]]:
+        """Separate a list of issues into errors and warnings.
+
+        Parameters:
+            issues_list (list[dict]): A list of issue dictionaries, each containing a 'severity' key.
+
+        Returns:
+            tuple[list[dict], list[dict]]: A tuple of (errors, warnings) where errors contains
+                issues with severity <= ErrorSeverity.ERROR and warnings contains issues with
+                severity >= ErrorSeverity.WARNING.
+        """
+        errors = [issue for issue in issues_list if issue["severity"] <= ErrorSeverity.ERROR]
+        warnings = [issue for issue in issues_list if issue["severity"] >= ErrorSeverity.WARNING]
+        return errors, warnings
+
     @staticmethod
     def format_error(error_type: str, *args, actual_error=None, **kwargs) -> list[dict]:
         """Format an error based on the parameters, which vary based on what type of error this is.

hed/scripts/schema_script_util.py

@@ -32,7 +32,7 @@ def _is_prerelease_partner(base_schema) -> bool:
     return hed_cache.get_hed_version_path(with_standard, check_prerelease=False) is None
 
 
-def validate_schema_object(base_schema, schema_name):
+def validate_schema_object(base_schema, schema_name, check_warnings=True):
     """Validate a schema object by checking non-warning compliance and roundtrip conversion.
 
     Tests the schema for non-warning compliance issues and validates that it can be successfully
@@ -41,14 +41,16 @@ def validate_schema_object(base_schema, schema_name):
     Parameters:
         base_schema (HedSchema): The schema object to validate.
         schema_name (str): The name/path of the schema for error reporting.
+        check_warnings (bool): If True, include warnings in the validation. Default is True.
 
     Returns:
         list: A list of validation issue strings. Empty if no issues found.
     """
     validation_issues = []
     try:
         issues = base_schema.check_compliance()
-        issues = [issue for issue in issues if issue.get("severity", ErrorSeverity.ERROR) == ErrorSeverity.ERROR]
+        if not check_warnings:
+            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)
@@ -74,15 +76,19 @@ def validate_schema_object(base_schema, schema_name):
     return validation_issues
 
 
-def validate_schema(file_path):
+def validate_schema(file_path, check_warnings=False):
     """Validate a schema file, ensuring it can save/load and passes validation.
 
     Loads the schema from file, checks the file extension is lowercase,
-    and validates the schema object for compliance and roundtrip conversion.
+    and validates the schema object for compliance errors and roundtrip conversion.
+    Does not check for warnings.
 
     Parameters:
         file_path (str): The path to the schema file to validate.
-
+            If loading a TSV file, this should be a single filename where:
+            Template: basename.tsv, where files are named basename_Struct.tsv, basename_Tag.tsv, etc.
+            Alternatively, you can point to a directory containing the .tsv files.
+        check_warnings (bool): If True, include warnings in the validation. Default is False.
     Returns:
         list: A list of validation issue strings. Empty if no issues found.
     """
@@ -96,7 +102,7 @@ def validate_schema(file_path):
     validation_issues = []
     try:
         base_schema = load_schema(file_path)
-        validation_issues = validate_schema_object(base_schema, file_path)
+        validation_issues = validate_schema_object(base_schema, file_path, check_warnings=check_warnings)
     except HedFileError as e:
         print(f"Saving/loading error: {file_path} {e.message}")
         error_text = e.message

tests/errors/test_error_reporter.py

@@ -299,3 +299,43 @@ def test_get_code_counts(self):
         result_with_missing = ErrorHandler.get_code_counts(issues_with_missing_code)
         expected_with_missing = {"VALID_CODE": 2, "UNKNOWN": 1}  # Default for missing code
         self.assertEqual(result_with_missing, expected_with_missing)
+
+
+class TestSeparateIssues(unittest.TestCase):
+    """Tests for ErrorHandler.separate_issues."""
+
+    @staticmethod
+    def _make_issue(severity):
+        return {"severity": severity, "message": "test"}
+
+    def test_empty_list(self):
+        errors, warnings = ErrorHandler.separate_issues([])
+        self.assertEqual(errors, [])
+        self.assertEqual(warnings, [])
+
+    def test_only_errors(self):
+        issues = [self._make_issue(ErrorSeverity.ERROR), self._make_issue(ErrorSeverity.ERROR)]
+        errors, warnings = ErrorHandler.separate_issues(issues)
+        self.assertEqual(len(errors), 2)
+        self.assertEqual(len(warnings), 0)
+
+    def test_only_warnings(self):
+        issues = [self._make_issue(ErrorSeverity.WARNING), self._make_issue(ErrorSeverity.WARNING)]
+        errors, warnings = ErrorHandler.separate_issues(issues)
+        self.assertEqual(len(errors), 0)
+        self.assertEqual(len(warnings), 2)
+
+    def test_mixed(self):
+        issues = [
+            self._make_issue(ErrorSeverity.ERROR),
+            self._make_issue(ErrorSeverity.WARNING),
+            self._make_issue(ErrorSeverity.ERROR),
+        ]
+        errors, warnings = ErrorHandler.separate_issues(issues)
+        self.assertEqual(len(errors), 2)
+        self.assertEqual(len(warnings), 1)
+
+    def test_original_list_unchanged(self):
+        issues = [self._make_issue(ErrorSeverity.ERROR), self._make_issue(ErrorSeverity.WARNING)]
+        ErrorHandler.separate_issues(issues)
+        self.assertEqual(len(issues), 2)

tests/scripts/test_script_util.py

@@ -1,15 +1,18 @@
+import contextlib
+import copy
 import unittest
 import os
 import shutil
+
 from hed import load_schema_version
 from hed.scripts.schema_script_util import (
     add_extension,
     sort_base_schemas,
     validate_all_schema_formats,
     validate_schema,
+    validate_schema_object,
     validate_all_schemas,
 )
-import contextlib
 
 
 class TestAddExtension(unittest.TestCase):
@@ -265,3 +268,53 @@ def test_uppercase_extension_policy_enforcement(self):
             # Clean up
             if os.path.exists(uppercase_file):
                 os.remove(uppercase_file)
+
+
+class TestCheckWarnings(unittest.TestCase):
+    """Tests for the check_warnings parameter in validate_schema_object and validate_schema."""
+
+    @classmethod
+    def setUpClass(cls):
+        clean = load_schema_version("8.3.0")
+        cls.clean_schema = clean
+        # Deep-copy so the cached shared instance is not mutated.
+        # Setting version to a future value triggers SCHEMA_PRERELEASE_VERSION_USED (warning only).
+        cls.warning_schema = copy.deepcopy(clean)
+        cls.warning_schema.header_attributes["version"] = "999.0.0"
+
+    def test_clean_schema_check_warnings_false(self):
+        """A fully compliant schema produces no issues with check_warnings=False."""
+        with contextlib.redirect_stdout(None):
+            issues = validate_schema_object(self.clean_schema, "test", check_warnings=False)
+        self.assertEqual(issues, [])
+
+    def test_clean_schema_check_warnings_true(self):
+        """A fully compliant schema produces no issues with check_warnings=True."""
+        with contextlib.redirect_stdout(None):
+            issues = validate_schema_object(self.clean_schema, "test", check_warnings=True)
+        self.assertEqual(issues, [])
+
+    def test_warning_schema_check_warnings_true_reports_issues(self):
+        """A prerelease version generates a warning that is reported when check_warnings=True."""
+        with contextlib.redirect_stdout(None):
+            issues = validate_schema_object(self.warning_schema, "test", check_warnings=True)
+        self.assertTrue(issues, "Expected at least one issue for prerelease version warning")
+
+    def test_warning_schema_check_warnings_false_suppresses_warnings(self):
+        """Warnings are suppressed and validation passes when check_warnings=False."""
+        with contextlib.redirect_stdout(None):
+            issues = validate_schema_object(self.warning_schema, "test", check_warnings=False)
+        self.assertEqual(issues, [])
+
+    def test_validate_schema_default_is_warnings_false(self):
+        """validate_schema default check_warnings=False matches explicit False."""
+        schema_path = os.path.join(
+            os.path.dirname(os.path.dirname(__file__)),
+            "data",
+            "schema_tests",
+            "HED8.2.0.mediawiki",
+        )
+        with contextlib.redirect_stdout(None):
+            default_issues = validate_schema(schema_path)
+            explicit_issues = validate_schema(schema_path, check_warnings=False)
+        self.assertEqual(default_issues, explicit_issues)