Merge pull request #61 from datakind/Validation-Errors

fix: output specific validation errors

Author: Mesh-ach

src/webapp/validation.py

@@ -5,7 +5,7 @@
 import csv
 
 from collections import Counter
-from typing import Final, Any
+from typing import Final, Any, NamedTuple
 
 from .utilities import SchemaType
 
@@ -260,6 +260,12 @@
 }
 
 
+class ColumnValidationResult(NamedTuple):
+    is_valid: bool
+    unexpected_columns: list[str]
+    missing_required_columns: list[str]
+
+
 def validate_file(filename: str, allowed_types: set[SchemaType]) -> set[SchemaType]:
     """Validates given a filename."""
     with open(filename) as f:
@@ -299,26 +305,55 @@ def get_col_names(f: Any) -> Any:
 
 def detect_file_type(col_names: list[str]) -> set[SchemaType]:
     """Returns all schemas that match for a list of col names."""
-    res = set()
-    for schema, schema_cols in SCHEMA_TYPE_TO_COLS.items():
+    matches = set()
+    errors = {}
+
+    for schema, expected_cols in SCHEMA_TYPE_TO_COLS.items():
         optional_cols = SCHEMA_TYPE_TO_OPTIONAL_COLS[schema]
-        if valid_subset_lists(schema_cols, col_names, optional_cols):
-            res.add(schema)
-    if not res:
-        # If it doesn't match any, it will match unknown.
-        res.add(SchemaType.UNKNOWN)
-    return res
+        result = valid_subset_lists(expected_cols, col_names, optional_cols)
+
+        if result.is_valid:
+            matches.add(schema)
+        else:
+            errors[schema.name] = result
+
+    if matches:
+        return matches
+
+    error_msgs = []
+    for schema_name, res in errors.items():
+        msg = f"\nSchema: {schema_name}"
+        if res.unexpected_columns:
+            msg += f"\n Unexpected columns: {res.unexpected_columns}"
+        if res.missing_required_columns:
+            msg += f"\n Missing required columns: {res.missing_required_columns}"
+        error_msgs.append(msg)
+
+    raise ValueError(
+        "No valid schema matched. Details of mismatches:\n" + "\n".join(error_msgs)
+    )
 
 
 def valid_subset_lists(
-    superset_list: list[str], subset_list: list[str], optional_list: list[str]
-) -> bool:
+    expected: list[str], actual: list[str], optional_list: list[str]
+) -> ColumnValidationResult:
     """Checks if the subset_list is a subset of or equivalent to superset_list. And if so,
     whether the missing values are all present in the optional list. This method disregards order
     but cares about duplicates."""
     # Checks if any value in subset list is NOT present in superset list.
-    if Counter(subset_list) - Counter(superset_list):
-        # This is not a valid state, users should not be passing in unrecognized columns.
-        return False
-    missing_vals = Counter(superset_list) - Counter(subset_list)
-    return not Counter(missing_vals) - Counter(optional_list)
+    expected_counter = Counter(expected)
+    actual_counter = Counter(actual)
+
+    unexpected = list((actual_counter - expected_counter).elements())
+
+    # Columns expected but missing (excluding optional)
+    missing_total = list((expected_counter - actual_counter).elements())
+    missing_required = [col for col in missing_total if col not in optional_list]
+
+    is_valid = not unexpected and not missing_required
+
+    return ColumnValidationResult(
+        is_valid=is_valid,
+        unexpected_columns=unexpected,
+        missing_required_columns=missing_required,
+    )

src/webapp/validation_test.py

@@ -19,16 +19,25 @@ def test_get_col_names():
 
 
 def test_valid_subset_lists():
-    """Testing valid subset checking."""
+    """Testing valid subset checking with detailed results."""
     list_a = [1, 2, 3, 4, 5, 6]
     list_b = [1, 2, 3, 4, 6]
     list_c = [5]
     list_d = [3, 4]
-    assert valid_subset_lists(list_a, list_b, list_c)
-    # Missing value is not in the optional list.
-    assert not valid_subset_lists(list_a, list_b, list_d)
-    # Subset has an additional element not found in superset.
-    assert not valid_subset_lists(list_b, list_a, list_c)
+
+    result1 = valid_subset_lists(list_a, list_b, list_c)
+    assert result1.is_valid
+    assert result1.unexpected_columns == []
+    assert result1.missing_required_columns == []
+
+    result2 = valid_subset_lists(list_a, list_b, list_d)
+    assert not result2.is_valid
+    assert result2.unexpected_columns == []
+    assert result2.missing_required_columns == [5]
+
+    result3 = valid_subset_lists(list_b, list_a, list_c)
+    assert not result3.is_valid
+    assert result3.unexpected_columns == [5]
 
 
 def test_detect_file_type():
@@ -45,7 +54,9 @@ def test_detect_file_type():
     with open("src/webapp/test_files/cohort_pdp.csv", encoding="utf-8") as f:
         assert detect_file_type(get_col_names(f)) == {SchemaType.PDP_COHORT}
     with open("src/webapp/test_files/test_upload.csv", encoding="utf-8") as f:
-        assert detect_file_type(get_col_names(f)) == {SchemaType.UNKNOWN}
+        with pytest.raises(ValueError) as err:
+            detect_file_type(get_col_names(f))
+        assert "No valid schema matched" in str(err.value)
     with open("src/webapp/test_files/malformed.csv", encoding="utf-8") as f:
         with pytest.raises(ValueError) as err:
             detect_file_type(get_col_names(f))
@@ -75,9 +86,5 @@ def test_validate_file():
         validate_file(
             "src/webapp/test_files/test_upload.csv", [SchemaType.SST_PDP_FINANCE]
         )
-    assert str(err.value) == "Some file schema/columns are not recognized"
-    with pytest.raises(ValueError) as err:
-        validate_file(
-            "src/webapp/test_files/malformed.csv", [SchemaType.SST_PDP_FINANCE]
-        )
-    assert str(err.value) == "CSV file malformed: Could not determine delimiter"
+    assert "No valid schema matched." in str(err.value)
+    assert "Unexpected columns" in str(err.value)