feat!: remove require at least one expression be passed to lazyframe select and lazyframe.with_columns, remove lazyframe.clone (#2206)

Author: MarcoGorelli

docs/api-reference/lazyframe.md

@@ -4,7 +4,6 @@
     handler: python
     options:
       members:
-        - clone
         - collect
         - collect_schema
         - columns

narwhals/_dask/dataframe.py

@@ -94,8 +94,6 @@ def collect(
         backend: Implementation | None,
         **kwargs: Any,
     ) -> CompliantDataFrame[Any]:
-        import pandas as pd
-
         result = self._native_frame.compute(**kwargs)
 
         if backend is None or backend is Implementation.PANDAS:
@@ -162,15 +160,6 @@ def aggregate(self: Self, *exprs: DaskExpr) -> Self:
 
     def select(self: Self, *exprs: DaskExpr) -> Self:
         new_series = evaluate_exprs(self, *exprs)
-
-        if not new_series:
-            # return empty dataframe, like Polars does
-            return self._from_native_frame(
-                dd.from_pandas(
-                    pd.DataFrame(), npartitions=self._native_frame.npartitions
-                ),
-            )
-
         df = select_columns_by_name(
             self._native_frame.assign(**dict(new_series)),
             [s[0] for s in new_series],

narwhals/_dask/namespace.py

@@ -107,17 +107,10 @@ def func(df: DaskLazyFrame) -> list[dx.Series]:
 
     def len(self: Self) -> DaskExpr:
         def func(df: DaskLazyFrame) -> list[dx.Series]:
-            if not df.columns:
-                return [
-                    dd.from_pandas(
-                        pd.Series([0], name="len"),
-                        npartitions=df._native_frame.npartitions,
-                    )
-                ]
+            # We don't allow dataframes with 0 columns, so `[0]` is safe.
             return [df._native_frame[df.columns[0]].size.to_series()]
 
-        # coverage bug? this is definitely hit
-        return DaskExpr(  # pragma: no cover
+        return DaskExpr(
             func,
             depth=0,
             function_name="len",

narwhals/_duckdb/dataframe.py

@@ -151,9 +151,6 @@ def select(
         *exprs: DuckDBExpr,
     ) -> Self:
         new_columns_map = evaluate_exprs(self, *exprs)
-        if not new_columns_map:
-            # TODO(marco): return empty relation with 0 columns?
-            return self._from_native_frame(self._native_frame.limit(0))
         return self._from_native_frame(
             self._native_frame.select(*(val.alias(col) for col, val in new_columns_map)),
         )

narwhals/_spark_like/dataframe.py

@@ -29,7 +29,6 @@
     import pyarrow as pa
     from sqlframe.base.column import Column
     from sqlframe.base.dataframe import BaseDataFrame
-    from sqlframe.base.session import _BaseSession
     from sqlframe.base.window import Window
     from typing_extensions import Self
     from typing_extensions import TypeAlias
@@ -41,7 +40,6 @@
     from narwhals.utils import Version
 
     SQLFrameDataFrame = BaseDataFrame[Any, Any, Any, Any, Any]
-    SQLFrameSession = _BaseSession[Any, Any, Any, Any, Any, Any, Any]
 
 Incomplete: TypeAlias = Any  # pragma: no cover
 """Marker for working code that fails type checking."""
@@ -90,14 +88,6 @@ def _Window(self: Self) -> type[Window]:  # noqa: N802
         else:
             return import_window(self._implementation)
 
-    @property
-    def _session(self: Self) -> SQLFrameSession:
-        if TYPE_CHECKING:
-            return self._native_frame.session
-        if self._implementation is Implementation.PYSPARK:
-            return self._native_frame.sparkSession
-        return self._native_frame.session
-
     def __native_namespace__(self: Self) -> ModuleType:  # pragma: no cover
         return self._implementation.to_native_namespace()
 
@@ -240,13 +230,6 @@ def select(
         *exprs: SparkLikeExpr,
     ) -> Self:
         new_columns = evaluate_exprs(self, *exprs)
-
-        if not new_columns:
-            # return empty dataframe, like Polars does
-            schema = self._native_dtypes.StructType([])
-            spark_df = self._session.createDataFrame([], schema)
-            return self._from_native_frame(spark_df)
-
         new_columns_list = [col.alias(col_name) for (col_name, col) in new_columns]
         return self._from_native_frame(self._native_frame.select(*new_columns_list))
 

narwhals/dataframe.py

@@ -278,9 +278,6 @@ def join(
             )
         )
 
-    def clone(self: Self) -> Self:
-        return self._from_compliant_dataframe(self._compliant_frame.clone())
-
     def gather_every(self: Self, n: int, offset: int = 0) -> Self:
         return self._from_compliant_dataframe(
             self._compliant_frame.gather_every(n=n, offset=offset)
@@ -1875,7 +1872,7 @@ def clone(self: Self) -> Self:
         Returns:
             An identical copy of the original dataframe.
         """
-        return super().clone()
+        return self._from_compliant_dataframe(self._compliant_frame.clone())
 
     def gather_every(self: Self, n: int, offset: int = 0) -> Self:
         r"""Take every nth row in the DataFrame and return as a new DataFrame.
@@ -2520,6 +2517,9 @@ def with_columns(
             |└───────┴──────────────┴───────┘|
             └────────────────────────────────┘
         """
+        if not exprs and not named_exprs:
+            msg = "At least one expression must be passed to LazyFrame.with_columns"
+            raise ValueError(msg)
         return super().with_columns(*exprs, **named_exprs)
 
     def select(
@@ -2561,6 +2561,9 @@ def select(
             |└───────┴──────────┘|
             └────────────────────┘
         """
+        if not exprs and not named_exprs:
+            msg = "At least one expression must be passed to LazyFrame.select"
+            raise ValueError(msg)
         return super().select(*exprs, **named_exprs)
 
     def rename(self: Self, mapping: dict[str, str]) -> Self:
@@ -3055,14 +3058,6 @@ def join_asof(
             suffix=suffix,
         )
 
-    def clone(self: Self) -> Self:
-        r"""Create a copy of this DataFrame.
-
-        Returns:
-            An identical copy of the original LazyFrame.
-        """
-        return super().clone()
-
     def lazy(self: Self) -> Self:
         """Restrict available API methods to lazy-only ones.
 

tests/expr_and_series/len_test.py

@@ -36,7 +36,7 @@ def test_namespace_len(constructor: Constructor) -> None:
     assert_equal_data(df, expected)
     df = (
         nw.from_native(constructor({"a": [1, 2, 3], "b": [4, 5, 6]}))
-        .select()
+        .filter(nw.col("a") < 0)
         .select(nw.len(), a=nw.len())
     )
     expected = {"len": [0], "a": [0]}

tests/frame/clone_test.py

@@ -3,20 +3,18 @@
 import pytest
 
 import narwhals.stable.v1 as nw
-from tests.utils import Constructor
+from tests.utils import ConstructorEager
 from tests.utils import assert_equal_data
 
 
-def test_clone(request: pytest.FixtureRequest, constructor: Constructor) -> None:
-    if "dask" in str(constructor):
-        request.applymarker(pytest.mark.xfail)
-    if ("pyspark" in str(constructor)) or "duckdb" in str(constructor):
-        request.applymarker(pytest.mark.xfail)
-    if "pyarrow_table" in str(constructor):
+def test_clone(
+    request: pytest.FixtureRequest, constructor_eager: ConstructorEager
+) -> None:
+    if "pyarrow_table" in str(constructor_eager):
         request.applymarker(pytest.mark.xfail)
 
     expected = {"a": [1, 2], "b": [3, 4]}
-    df = nw.from_native(constructor(expected))
+    df = nw.from_native(constructor_eager(expected), eager_only=True)
     df_clone = df.clone()
     assert df is not df_clone
     assert df._compliant_frame is not df_clone._compliant_frame

tests/frame/select_test.py

@@ -29,11 +29,9 @@ def test_select(constructor: Constructor) -> None:
     assert_equal_data(result, expected)
 
 
-def test_empty_select(constructor: Constructor, request: pytest.FixtureRequest) -> None:
-    if "duckdb" in str(constructor) or "sqlframe" in str(constructor):
-        request.applymarker(pytest.mark.xfail)
-    result = nw.from_native(constructor({"a": [1, 2, 3]})).lazy().select()
-    assert result.collect().shape == (0, 0)
+def test_empty_select(constructor_eager: ConstructorEager) -> None:
+    result = nw.from_native(constructor_eager({"a": [1, 2, 3]}), eager_only=True).select()
+    assert result.shape == (0, 0)
 
 
 def test_non_string_select() -> None:

tests/frame/with_columns_test.py

@@ -7,6 +7,7 @@
 import narwhals.stable.v1 as nw
 from tests.utils import PYARROW_VERSION
 from tests.utils import Constructor
+from tests.utils import ConstructorEager
 from tests.utils import assert_equal_data
 
 
@@ -30,13 +31,22 @@ def test_with_columns_order(constructor: Constructor) -> None:
     assert_equal_data(result, expected)
 
 
-def test_with_columns_empty(constructor: Constructor) -> None:
+def test_with_columns_empty(constructor_eager: ConstructorEager) -> None:
     data = {"a": [1, 3, 2], "b": [4, 4, 6], "z": [7.0, 8.0, 9.0]}
-    df = nw.from_native(constructor(data))
+    df = nw.from_native(constructor_eager(data))
     result = df.select().with_columns()
     assert_equal_data(result, {})
 
 
+def test_select_with_columns_empty_lazy(constructor: Constructor) -> None:
+    data = {"a": [1, 3, 2], "b": [4, 4, 6], "z": [7.0, 8.0, 9.0]}
+    df = nw.from_native(constructor(data)).lazy()
+    with pytest.raises(ValueError, match="At least one"):
+        df.with_columns()
+    with pytest.raises(ValueError, match="At least one"):
+        df.select()
+
+
 def test_with_columns_order_single_row(constructor: Constructor) -> None:
     data = {"a": [1, 3, 2], "b": [4, 4, 6], "z": [7.0, 8.0, 9.0], "i": [0, 1, 2]}
     df = nw.from_native(constructor(data)).filter(nw.col("i") < 1).drop("i")