chore: planning fancy binary combination expansion

Author: dangotbanned

narwhals/_plan/_expansion.py

@@ -265,7 +265,7 @@ def _expand_only(self, child: ExprIR, /) -> ExprIR:
             raise MultiOutputExpressionError(msg)
         return first
 
-    # TODO @dangotbanned: It works, but all this class-specfic branching belongs in the classes themselves
+    # TODO @dangotbanned: It works, but all this class-specific branching belongs in the classes themselves
     def _expand_combination(self, origin: Combination, /) -> Iterator[Combination]:
         changes: dict[str, Any] = {}
         if isinstance(origin, (ir.WindowExpr, ir.Filter, ir.SortBy)):
@@ -281,6 +281,10 @@ def _expand_combination(self, origin: Combination, /) -> Iterator[Combination]:
             replaced = common.replace(origin, **changes)
             for root in self._expand_recursive(replaced.expr):
                 yield common.replace(replaced, expr=root)
+
+        # 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
         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

@@ -46,6 +46,13 @@ 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):

tests/plan/expr_parsing_test.py

@@ -208,13 +208,15 @@ def test_filtration_over() -> None:
 
 
 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"
         "                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^"
@@ -225,6 +227,7 @@ def test_invalid_binary_expr_multi() -> None:
         "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()