From fb24ff50b382d9cfa75ebee5567b6ee41e4b6a97 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 20 Jul 2026 10:14:33 +0200 Subject: [PATCH] =?UTF-8?q?test(v1):=20co-locate=20=C2=A76=20masked-addend?= =?UTF-8?q?=20tests=20and=20pin=20fallback-guard=20messages?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up polish for the #717 "polish the tests" checklist: - Move the four masked-addend §6 tests (absence propagation of a fully masked term in a sum) out of test_linear_expression.py into TestAbsencePropagation in test_legacy_violations.py, where the rest of the §6 coverage lives. They now share one `ab_all_masked` fixture instead of rebuilding an inline two-variable model four times. - Add `match="only supported for"` to the four bare `pytest.raises(ValueError)` guards on the `use_fallback=True` DataFrame-grouper path, so they assert *why* the fallback raises rather than accepting any ValueError. The fifth guard (issue #351) keeps its precise `(KeyError, IndexError)` — its message is an incidental numpy index error, brittle to pin. No behaviour change; test_legacy_violations.py 294 passed, test_linear_expression.py green, ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/test_legacy_violations.py | 55 +++++++++++++++++++++++++++ test/test_linear_expression.py | 69 ++-------------------------------- 2 files changed, 59 insertions(+), 65 deletions(-) diff --git a/test/test_legacy_violations.py b/test/test_legacy_violations.py index fe1af6ea4..ec35e0a0a 100644 --- a/test/test_legacy_violations.py +++ b/test/test_legacy_violations.py @@ -1033,6 +1033,15 @@ def xs(self, x: Variable) -> Variable: # x.shift(time=1) → absent at time=0, present elsewhere. return x.shift(time=1) + @pytest.fixture + def ab_all_masked(self, m: Model) -> tuple[Variable, Variable, xr.DataArray]: + """Two present variables over a shared dim plus an all-False mask.""" + t = pd.Index(range(3), name="time") + a = m.add_variables(name="a", coords=[t]) + b = m.add_variables(name="b", coords=[t]) + mask = xr.DataArray(False, coords=[t]) + return a, b, mask + @pytest.mark.v1 def test_to_linexpr_marks_absent_with_nan_const(self, xs: Variable) -> None: """ @@ -1261,6 +1270,52 @@ def test_legacy_quadratic_collapses_absent( assert not bool(quad.isnull().values[0]) assert not np.isnan(quad.coeffs.values[0]).any() + # --- masked-addend absence (a fully masked term in a sum) ----------- + # Moved from test_linear_expression.py to co-locate §6 coverage. + + @pytest.mark.v1 + def test_masked_addend_absorbs_sum( + self, ab_all_masked: tuple[Variable, Variable, xr.DataArray] + ) -> None: + """§6: a fully masked addend absorbs the whole sum — no live terms.""" + a, b, mask = ab_all_masked + expr = a + (b * 1).where(mask) + assert expr.variable_names == set() + assert expr.isnull().all() + + @pytest.mark.v1 + def test_simplify_absent_expression_has_no_terms( + self, ab_all_masked: tuple[Variable, Variable, xr.DataArray] + ) -> None: + """§6: absence propagates, so simplify() leaves nothing to keep.""" + a, b, mask = ab_all_masked + expr = (a + b.where(mask)).simplify() + assert expr.nterm == 0 + assert expr.isnull().all() + + @pytest.mark.legacy + def test_legacy_masked_addend_keeps_dead_terms( + self, ab_all_masked: tuple[Variable, Variable, xr.DataArray] + ) -> None: + """Legacy: the masked addend's terms turn dead; the live one remains.""" + a, b, mask = ab_all_masked + expr = a + (b * 1).where(mask) + assert expr.nterm == 2 + assert expr.variable_names == {"a"} + + expr = (b * 1).where(mask) + assert expr.nterm == 1 + assert expr.variable_names == set() + + @pytest.mark.legacy + def test_legacy_simplify_drops_masked_addend( + self, ab_all_masked: tuple[Variable, Variable, xr.DataArray] + ) -> None: + """Legacy: simplify() drops the masked addend's dead terms.""" + a, b, mask = ab_all_masked + expr = (a + b.where(mask)).simplify() + assert expr.nterm == 1 + class TestFillnaResolves: """§7 — fillna()/.where() are how the caller resolves an absent slot.""" diff --git a/test/test_linear_expression.py b/test/test_linear_expression.py index ae80839fb..c230b77ae 100644 --- a/test/test_linear_expression.py +++ b/test/test_linear_expression.py @@ -1906,7 +1906,7 @@ def test_linear_expression_groupby_with_dataframe(v: Variable) -> None: groups = pd.DataFrame( {"a": [1] * 10 + [2] * 10, "b": list(range(4)) * 5}, index=v.indexes["dim_2"] ) - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="only supported for"): expr.groupby(groups).sum(use_fallback=True) with pytest.warns(LinopySemanticsWarning, match=r"stacked `group` MultiIndex"): grouped = expr.groupby(groups).sum() @@ -1924,7 +1924,7 @@ def test_linear_expression_groupby_with_dataframe_v1(v: Variable) -> None: groups = pd.DataFrame( {"a": [1] * 10 + [2] * 10, "b": list(range(4)) * 5}, index=v.indexes["dim_2"] ) - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="only supported for"): expr.groupby(groups).sum(use_fallback=True) grouped = expr.groupby(groups).sum() assert not isinstance(grouped.indexes["group"], pd.MultiIndex) @@ -1943,7 +1943,7 @@ def test_linear_expression_groupby_with_dataframe_with_same_group_name( {"dim_2": [1] * 10 + [2] * 10, "b": list(range(4)) * 5}, index=v.indexes["dim_2"], ) - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="only supported for"): expr.groupby(groups).sum(use_fallback=True) with pytest.warns(LinopySemanticsWarning, match=r"stacked `group` MultiIndex"): grouped = expr.groupby(groups).sum() @@ -1974,7 +1974,7 @@ def test_linear_expression_groupby_with_dataframe_on_multiindex(u: Variable) -> expr = 1 * u n = len(u.data["dim_3"]) groups = pd.DataFrame({"a": [1] * n}, index=u.indexes["dim_3"]) - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="only supported for"): expr.groupby(groups).sum(use_fallback=True) with pytest.warns(LinopySemanticsWarning, match=r"stacked `group` MultiIndex"): grouped = expr.groupby(groups).sum() @@ -2869,39 +2869,6 @@ def test_variable_names() -> None: assert expr.variable_names == {"a"} -@pytest.mark.legacy -def test_variable_names_masked_addend_legacy() -> None: - # Legacy: a fully masked addend's terms turn dead; the live variable remains. - m = Model() - time = pd.Index(range(3), name="time") - a = m.add_variables(name="a", coords=[time]) - b = m.add_variables(name="b", coords=[time]) - mask = xr.DataArray(False, coords=[time]) - - expr = a + (b * 1).where(mask) - assert expr.nterm == 2 - assert expr.variable_names == {"a"} - - expr = (b * 1).where(mask) - assert expr.nterm == 1 - assert expr.variable_names == set() - - -@pytest.mark.v1 -def test_variable_names_masked_addend_v1() -> None: - # v1 (section 6): absence propagates — a sum with a fully masked addend - # has no live terms anywhere. - m = Model() - time = pd.Index(range(3), name="time") - a = m.add_variables(name="a", coords=[time]) - b = m.add_variables(name="b", coords=[time]) - mask = xr.DataArray(False, coords=[time]) - - expr = a + (b * 1).where(mask) - assert expr.variable_names == set() - assert expr.isnull().all() - - def test_nterm() -> None: m = Model() time = pd.Index(range(3), name="time") @@ -2921,34 +2888,6 @@ def test_nterm() -> None: assert expr.nterm == 2 -@pytest.mark.legacy -def test_nterm_simplify_collapses_fully_masked_addend() -> None: - # Legacy: a fully masked addend contributes dead terms that simplify() drops. - m = Model() - time = pd.Index(range(3), name="time") - all_false = xr.DataArray(False, coords=[time]) - a = m.add_variables(name="a", coords=[time]) - b = m.add_variables(name="b", coords=[time]) - - expr = (a + b.where(all_false)).simplify() - assert expr.nterm == 1 - - -@pytest.mark.v1 -def test_nterm_simplify_absent_expression_has_no_terms() -> None: - # v1 (section 6): absence propagates — a fully masked addend absorbs the - # whole sum, so nothing is left to simplify. - m = Model() - time = pd.Index(range(3), name="time") - all_false = xr.DataArray(False, coords=[time]) - a = m.add_variables(name="a", coords=[time]) - b = m.add_variables(name="b", coords=[time]) - - expr = (a + b.where(all_false)).simplify() - assert expr.nterm == 0 - assert expr.isnull().all() - - class TestJoinParameter: @pytest.fixture def m2(self) -> Model: