adapt old BinaryExpr tests and make them fail

Author: dangotbanned

narwhals/_plan/_expansion.py

@@ -284,7 +284,12 @@ def _expand_combination(self, origin: Combination, /) -> Iterator[Combination]:
 
         # TODO @dangotbanned: Relax `BinaryExpr.right`
         # - https://github.com/narwhals-dev/narwhals/pull/3233#discussion_r2472757798
-        # - https://github.com/narwhals-dev/narwhals/pull/3233#discussion_r2473810664
+        # - https://github.com/narwhals-dev/narwhals/pull/3233#discussion_r2473810664=
+        # NOTE: Only need to raise if outputs are not:
+        # - 1:1
+        # - M:1
+        # - 1:M
+        # - N:N
         elif isinstance(origin, ir.BinaryExpr):
             binary = origin.__replace__(right=self._expand_only(origin.right))
             for root in self._expand_recursive(binary.left):

narwhals/_plan/expressions/operators.py

@@ -7,7 +7,6 @@
 from narwhals._plan._immutable import Immutable
 from narwhals._plan.exceptions import (
     binary_expr_length_changing_error,
-    binary_expr_multi_output_error,
     binary_expr_shape_error,
 )
 
@@ -46,15 +45,6 @@ def to_binary_expr(
     ) -> BinaryExpr[LeftT, Self, RightT]:
         from narwhals._plan.expressions.expr import BinaryExpr
 
-        # TODO @dangotbanned: Leave validating this until actually expanding
-        # Only need to raise if outputs are not:
-        # - 1:1
-        # - M:1
-        # - 1:M
-        # - N:N
-        # https://github.com/narwhals-dev/narwhals/pull/3233#discussion_r2473810664
-        if right.meta.has_multiple_outputs():
-            raise binary_expr_multi_output_error(left, self, right)
         if _is_filtration(left):
             if _is_filtration(right):
                 raise binary_expr_length_changing_error(left, self, right)

tests/plan/expr_expansion_test.py

@@ -9,7 +9,11 @@
 from narwhals import _plan as nwp
 from narwhals._plan import expressions as ir, selectors as ncs
 from narwhals._utils import zip_strict
-from narwhals.exceptions import ColumnNotFoundError, DuplicateError
+from narwhals.exceptions import (
+    ColumnNotFoundError,
+    DuplicateError,
+    MultiOutputExpressionError,
+)
 from tests.plan.utils import Frame, assert_expr_ir_equal, named_ir, re_compile
 
 if TYPE_CHECKING:
@@ -596,3 +600,41 @@ def test_prepare_projection_index_error(
         ),
     ):
         df_1.project(into_exprs)
+
+
+@pytest.mark.xfail(
+    reason="TODO: binary_expr_combination", raises=MultiOutputExpressionError
+)
+def test_expand_binary_expr_combination(df_1: Frame) -> None:  # pragma: no cover
+    three_to_three = nwp.nth(range(3)) * nwp.nth(3, 4, 5).max()
+
+    expecteds = [
+        named_ir("a", nwp.col("a") * nwp.col("d")),
+        named_ir("b", nwp.col("b") * nwp.col("e")),
+        named_ir("c", nwp.col("c") * nwp.col("f")),
+    ]
+    actuals = df_1.project(three_to_three)
+    for actual, expected in zip_strict(actuals, expecteds):
+        assert_expr_ir_equal(actual, expected)
+
+
+@pytest.mark.xfail(reason="TODO: Move fancy error message", raises=AssertionError)
+def test_expand_binary_expr_combination_invalid(df_1: Frame) -> None:  # pragma: no cover
+    pattern = re.escape(
+        "ncs.all() + ncs.by_name('b', 'c', require_all=True)\n"
+        "            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^"
+    )
+    all_to_two = nwp.all() + nwp.col("b", "c")
+    with pytest.raises(MultiOutputExpressionError, match=pattern):
+        df_1.project(all_to_two)
+
+    pattern = re.escape(
+        "ncs.by_name('a', 'b', 'c', require_all=True).abs().fill_null([lit(int: 0)]).round() * ncs.by_index([9, 10], require_all=True).cast(Int64).sort(asc)\n"
+        "                                                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^"
+    )
+    three_to_two = (
+        nwp.col("a", "b", "c").abs().fill_null(0).round(2)
+        * nwp.nth(9, 10).cast(nw.Int64).sort()
+    )
+    with pytest.raises(MultiOutputExpressionError, match=pattern):
+        df_1.project(three_to_two)

tests/plan/expr_parsing_test.py

@@ -20,7 +20,6 @@
     InvalidIntoExprError,
     InvalidOperationError,
     InvalidOperationError as LengthChangingExprError,
-    MultiOutputExpressionError,
     ShapeError,
 )
 from tests.plan.utils import assert_expr_ir_equal
@@ -207,33 +206,6 @@ def test_filtration_over() -> None:
         nwp.col("a").diff().drop_nulls().over("b", order_by="i")
 
 
-def test_invalid_binary_expr_multi() -> None:
-    # TODO @dangotbanned: Move to `expr_expansion_test`
-    pattern = re.escape(
-        "ncs.all() + ncs.by_name('b', 'c', require_all=True)\n"
-        "            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^"
-    )
-    with pytest.raises(MultiOutputExpressionError, match=pattern):
-        nwp.all() + nwp.col("b", "c")
-
-    # TODO @dangotbanned: Use as a positive case (3:3)
-    pattern = re.escape(
-        "ncs.by_index([1, 2, 3], require_all=True) * ncs.by_index([4, 5, 6], require_all=True).max()\n"
-        "                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^"
-    )
-    with pytest.raises(MultiOutputExpressionError, match=pattern):
-        nwp.nth(1, 2, 3) * nwp.nth(4, 5, 6).max()
-    pattern = re.escape(
-        "ncs.by_name('a', 'b', 'c', require_all=True).abs().fill_null([lit(int: 0)]).round() * ncs.by_index([9, 10], require_all=True).cast(Int64).sort(asc)\n"
-        "                                                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^"
-    )
-    # TODO @dangotbanned: Move to `expr_expansion_test`
-    with pytest.raises(MultiOutputExpressionError, match=pattern):
-        nwp.col("a", "b", "c").abs().fill_null(0).round(2) * nwp.nth(9, 10).cast(
-            nw.Int64()
-        ).sort()
-
-
 def test_invalid_binary_expr_length_changing() -> None:
     a = nwp.col("a")
     b = nwp.col("b")