Merge pull request #73 from UBC-MDS/yonas.test-fix

Enhance test documentaion for function compare contracts

Author: EduardoRasanmar

tests/unit/test_compare_contracts.py

@@ -1,5 +1,11 @@
 """
-Basic unit tests for compare_contracts.
+Unit tests for compare_contracts.
+
+The compare_contracts function should:
+- Identify schema drift (added/removed columns, dtype changes).
+- Identify constraint drift (range, category, missingness changes).
+- Validate input contracts and raise on invalid rules.
+- Report drift in a consistent shape and ordering.
 """
 
 import pytest
@@ -9,30 +15,44 @@
 
 
 def test_added_and_removed_columns():
-    """Detect added and removed columns between contracts."""
+    """Detects added/removed columns, which would break downstream schema.
+
+    We swap the single column name between contracts to ensure both the
+    addition and removal are reported in the correct direction.
+    """
     contract_a = Contract(columns={"age": ColumnRule(dtype="int")})
     contract_b = Contract(columns={"height": ColumnRule(dtype="int")})
 
     report = compare_contracts(contract_a, contract_b)
 
+    # New column appears only in contract_b; old column only in contract_a.
     assert report.added_columns == {"height"}
     assert report.removed_columns == {"age"}
     assert report.has_drift is True
 
 
 def test_dtype_change():
-    """Detect dtype changes for shared columns."""
+    """Detects dtype changes for shared columns, a common schema break.
+
+    We keep the column name but change its dtype to verify the change is
+    reported as an (old, new) pair.
+    """
     contract_a = Contract(columns={"age": ColumnRule(dtype="int")})
     contract_b = Contract(columns={"age": ColumnRule(dtype="float")})
 
     report = compare_contracts(contract_a, contract_b)
 
+    # Dtype changes are reported as (old, new).
     assert report.dtype_changes == {"age": ("int", "float")}
     assert report.has_drift is True
 
 
 def test_range_change():
-    """Detect min/max range changes for shared columns."""
+    """Detects numeric range drift when dtype stays the same.
+
+    We change the min bound for a float column and verify the column is
+    flagged in the range_changes set.
+    """
     contract_a = Contract(
         columns={"score": ColumnRule(dtype="float", min_value=0.0, max_value=1.0)}
     )
@@ -42,12 +62,17 @@ def test_range_change():
 
     report = compare_contracts(contract_a, contract_b)
 
+    # Same dtype but different bounds should trigger range drift.
     assert report.range_changes == {"score"}
     assert report.has_drift is True
 
 
 def test_category_and_missingness_changes():
-    """Detect category and missingness changes for shared columns."""
+    """Detects category and missingness drift for a shared categorical column.
+
+    We expand allowed values and relax max_missing_frac to confirm both
+    change types are captured in the report.
+    """
     contract_a = Contract(
         columns={
             "status": ColumnRule(
@@ -67,13 +92,18 @@ def test_category_and_missingness_changes():
 
     report = compare_contracts(contract_a, contract_b)
 
+    # Allowed values expansion and missingness change are recorded independently.
     assert report.category_changes == {"status"}
     assert report.missingness_changes == {"status": (0.05, 0.10)}
     assert report.has_drift is True
 
 
 def test_missingness_only_change():
-    """Detect missingness drift without other changes."""
+    """Detects missingness drift without implying other drift types.
+
+    We change only max_missing_frac and confirm dtype/range/category drift
+    remain empty.
+    """
     contract_a = Contract(
         columns={"age": ColumnRule(dtype="int", max_missing_frac=0.05)}
     )
@@ -83,14 +113,19 @@ def test_missingness_only_change():
 
     report = compare_contracts(contract_a, contract_b)
 
+    # Only missingness should change; all other categories remain empty.
     assert report.missingness_changes == {"age": (0.05, 0.10)}
     assert report.dtype_changes == {}
     assert report.range_changes == set()
     assert report.category_changes == set()
 
 
 def test_category_only_change():
-    """Detect category drift without other changes."""
+    """Detects category drift without implying other drift types.
+
+    We expand allowed_values and confirm dtype/range/missingness drift
+    stay empty.
+    """
     contract_a = Contract(
         columns={"status": ColumnRule(dtype="category", allowed_values={"new", "old"})}
     )
@@ -104,14 +139,19 @@ def test_category_only_change():
 
     report = compare_contracts(contract_a, contract_b)
 
+    # Only categories should change; other buckets remain empty.
     assert report.category_changes == {"status"}
     assert report.dtype_changes == {}
     assert report.range_changes == set()
     assert report.missingness_changes == {}
 
 
 def test_range_change_none_to_value():
-    """None to value for range triggers drift."""
+    """Detects range drift when optional bounds become defined.
+
+    We move min/max from None to numeric values to verify the column is
+    flagged as a range change.
+    """
     contract_a = Contract(
         columns={"score": ColumnRule(dtype="float", min_value=None, max_value=None)}
     )
@@ -121,11 +161,16 @@ def test_range_change_none_to_value():
 
     report = compare_contracts(contract_a, contract_b)
 
+    # Optional bounds become defined, which is a drift.
     assert report.range_changes == {"score"}
 
 
 def test_category_change_none_to_set():
-    """None to set for categories triggers drift."""
+    """Detects category drift when allowed_values becomes defined.
+
+    We move allowed_values from None to a set to confirm category drift
+    is reported.
+    """
     contract_a = Contract(
         columns={"status": ColumnRule(dtype="category", allowed_values=None)}
     )
@@ -135,11 +180,16 @@ def test_category_change_none_to_set():
 
     report = compare_contracts(contract_a, contract_b)
 
+    # Optional category set becomes defined, which is a drift.
     assert report.category_changes == {"status"}
 
 
 def test_none_to_none_no_drift():
-    """None to None for optional fields yields no drift."""
+    """Confirms no drift when optional fields stay None.
+
+    We keep min/max as None in both contracts and verify the report does
+    not flag a range or category change.
+    """
     contract_a = Contract(
         columns={"score": ColumnRule(dtype="float", min_value=None, max_value=None)}
     )
@@ -149,12 +199,16 @@ def test_none_to_none_no_drift():
 
     report = compare_contracts(contract_a, contract_b)
 
+    # Optional fields unchanged -> no drift reported.
     assert report.range_changes == set()
     assert report.category_changes == set()
 
 
 def test_dtype_change_blocks_range_drift():
-    """Range drift is not reported when dtype changes."""
+    """Ensures dtype changes suppress range drift for that column.
+
+    We change dtype and bounds to confirm only dtype drift is reported.
+    """
     contract_a = Contract(
         columns={"score": ColumnRule(dtype="int", min_value=0.0, max_value=10.0)}
     )
@@ -164,12 +218,16 @@ def test_dtype_change_blocks_range_drift():
 
     report = compare_contracts(contract_a, contract_b)
 
+    # Dtype change is reported; range changes are ignored when dtype differs.
     assert report.dtype_changes == {"score": ("int", "float")}
     assert report.range_changes == set()
 
 
 def test_dtype_change_blocks_category_drift():
-    """Category drift is not reported when dtype changes."""
+    """Ensures dtype changes suppress category drift for that column.
+
+    We change dtype and allowed_values to verify only dtype drift appears.
+    """
     contract_a = Contract(
         columns={"status": ColumnRule(dtype="category", allowed_values={"new", "old"})}
     )
@@ -183,20 +241,27 @@ def test_dtype_change_blocks_category_drift():
 
     report = compare_contracts(contract_a, contract_b)
 
+    # Dtype change is reported; category changes are ignored when dtype differs.
     assert report.dtype_changes == {"status": ("category", "string")}
     assert report.category_changes == set()
 
 
 def test_invalid_contract_type_raises_typeerror():
-    """Invalid contract types raise TypeError."""
+    """Validates inputs: non-Contract arguments should raise TypeError.
+
+    We pass a string in place of contract_a to ensure the guard triggers.
+    """
     contract_b = Contract(columns={"age": ColumnRule(dtype="int")})
 
     with pytest.raises(TypeError):
         compare_contracts("not-a-contract", contract_b)
 
 
 def test_invalid_missing_frac_raises_valueerror():
-    """Invalid missingness fraction raises ValueError."""
+    """Validates rule constraints: max_missing_frac must be within [0, 1].
+
+    We set max_missing_frac above 1.0 to confirm the validation raises.
+    """
     contract_a = Contract(
         columns={"age": ColumnRule(dtype="int", max_missing_frac=1.5)}
     )
@@ -207,7 +272,10 @@ def test_invalid_missing_frac_raises_valueerror():
 
 
 def test_non_columnrule_raises_typeerror():
-    """Non-ColumnRule entries raise TypeError."""
+    """Validates inputs: every column must map to a ColumnRule instance.
+
+    We inject a plain string for a column rule to ensure TypeError is raised.
+    """
     contract_a = Contract(columns={"age": "not-a-rule"})
     contract_b = Contract(columns={"age": ColumnRule(dtype="int")})
 
@@ -216,7 +284,10 @@ def test_non_columnrule_raises_typeerror():
 
 
 def test_non_numeric_missing_frac_raises_valueerror():
-    """Non-numeric max_missing_frac raises ValueError."""
+    """Validates rule constraints: max_missing_frac must be numeric.
+
+    We pass a non-numeric value and confirm the validation raises.
+    """
     contract_a = Contract(
         columns={"age": ColumnRule(dtype="int", max_missing_frac="high")}
     )
@@ -227,7 +298,11 @@ def test_non_numeric_missing_frac_raises_valueerror():
 
 
 def test_invalid_contract_b_raises_valueerror():
-    """Invalid contract_b triggers validation on the second contract."""
+    """Validates both contracts: contract_b must pass the same checks.
+
+    We put an invalid missingness fraction in contract_b to ensure its
+    validation is not skipped.
+    """
     contract_a = Contract(columns={"age": ColumnRule(dtype="int")})
     contract_b = Contract(
         columns={"age": ColumnRule(dtype="int", max_missing_frac=2.0)}
@@ -238,7 +313,10 @@ def test_invalid_contract_b_raises_valueerror():
 
 
 def test_min_greater_than_max_raises_valueerror():
-    """Invalid range bounds raise ValueError."""
+    """Validates numeric bounds: min_value cannot exceed max_value.
+
+    We set min_value > max_value to confirm the guard raises ValueError.
+    """
     contract_a = Contract(
         columns={"age": ColumnRule(dtype="int", min_value=10.0, max_value=1.0)}
     )
@@ -278,13 +356,20 @@ def test_min_greater_than_max_raises_valueerror():
     ],
 )
 def test_has_drift_true_for_any_nonempty_change(contract_a, contract_b):
-    """Any non-empty drift category flips has_drift to True."""
+    """Reports has_drift True for any non-empty drift category.
+
+    We parametrize over each drift type and confirm has_drift flips to True
+    whenever at least one drift bucket is populated.
+    """
     report = compare_contracts(contract_a, contract_b)
     assert report.has_drift is True
 
 
 def test_has_drift_false_for_no_drift():
-    """No drift yields has_drift False."""
+    """Reports has_drift False when contracts are identical.
+
+    We compare two identical contracts to ensure the summary flag is False.
+    """
     contract_a = Contract(columns={"age": ColumnRule(dtype="int")})
     contract_b = Contract(columns={"age": ColumnRule(dtype="int")})
 
@@ -294,7 +379,11 @@ def test_has_drift_false_for_no_drift():
 
 
 def test_multiple_columns_mixed_drift():
-    """Aggregate drift across multiple columns."""
+    """Aggregates drift across multiple columns and drift types.
+
+    We introduce one change per drift category to confirm all are captured
+    in a single report.
+    """
     contract_a = Contract(
         columns={
             "a": ColumnRule(dtype="int"),
@@ -315,6 +404,7 @@ def test_multiple_columns_mixed_drift():
 
     report = compare_contracts(contract_a, contract_b)
 
+    # Each drift category should reflect the intended column(s).
     assert report.added_columns == {"e"}
     assert report.removed_columns == set()
     assert report.dtype_changes == {"a": ("int", "float")}
@@ -324,7 +414,11 @@ def test_multiple_columns_mixed_drift():
 
 
 def test_missingness_reports_old_new_order():
-    """Missingness changes are reported as (old, new)."""
+    """Confirms missingness changes are ordered as (old, new).
+
+    We increase max_missing_frac and verify the tuple preserves old-to-new
+    ordering for downstream reporting.
+    """
     contract_a = Contract(
         columns={"age": ColumnRule(dtype="int", max_missing_frac=0.05)}
     )
@@ -338,7 +432,10 @@ def test_missingness_reports_old_new_order():
 
 
 def test_dtype_reports_old_new_order():
-    """Dtype changes are reported as (old, new)."""
+    """Confirms dtype changes are ordered as (old, new).
+
+    We change dtype and ensure the report preserves the old-to-new ordering.
+    """
     contract_a = Contract(columns={"age": ColumnRule(dtype="int")})
     contract_b = Contract(columns={"age": ColumnRule(dtype="float")})
 
@@ -348,7 +445,11 @@ def test_dtype_reports_old_new_order():
 
 
 def test_no_drift():
-    """No differences yields an empty report."""
+    """Confirms no drift yields empty buckets and has_drift False.
+
+    We keep all fields identical to verify each drift bucket is empty and
+    the summary flag remains False.
+    """
     contract_a = Contract(
         columns={"age": ColumnRule(dtype="int", min_value=0.0, max_value=120.0)}
     )