-
Notifications
You must be signed in to change notification settings - Fork 211
feat: Add backend argument to lazy
#1895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 13 commits
69e5ccc
cfb4904
89b0e47
835feec
9499383
0c339ff
b209c6c
c74778a
a379167
0c70307
cfca1a0
6444a7e
c92c5a0
b352f71
0292363
24c4ebf
999f3ed
66fe6a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -142,7 +142,14 @@ def drop(self: Self, columns: list[str], strict: bool) -> Self: # noqa: FBT001 | |||||
| selection = (col for col in self.columns if col not in columns_to_drop) | ||||||
| return self._from_native_frame(self._native_frame.select(*selection)) | ||||||
|
|
||||||
| def lazy(self: Self) -> Self: | ||||||
| def lazy(self: Self, *, backend: Implementation | None = None) -> Self: | ||||||
| # The `backend`` argument has no effect but we keep it here for | ||||||
| # backwards compatibility because in `narwhals.stable.v1` | ||||||
| # function `.from_native()` will return a DataFrame for DuckDB. | ||||||
|
|
||||||
| if backend is not None: # pragma: no cover | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think duckdb should be allowed?
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't think it needs to be - if this path only exists for v1 backwards-compatibility, then #1895 (comment) is something somebody could have written, but they wouldn't have passed a backend, so raising for |
||||||
| msg = "`backend` argument is not supported for DuckDB" | ||||||
| raise ValueError(msg) | ||||||
| return self | ||||||
|
Comment on lines
+145
to
153
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If it would, I feel like I rather raise if the passed
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the review, @FBruzzesi! I added the which otherwise made the tests fail for |
||||||
|
|
||||||
| def with_columns( | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |
| from narwhals.exceptions import ShapeError | ||
| from narwhals.schema import Schema | ||
| from narwhals.translate import to_native | ||
| from narwhals.utils import Implementation | ||
| from narwhals.utils import find_stacklevel | ||
| from narwhals.utils import flatten | ||
| from narwhals.utils import generate_repr | ||
|
|
@@ -50,7 +51,6 @@ | |
| from narwhals.typing import IntoExpr | ||
| from narwhals.typing import IntoFrame | ||
| from narwhals.typing import SizeUnit | ||
| from narwhals.utils import Implementation | ||
|
|
||
| PS = ParamSpec("PS") | ||
|
|
||
|
|
@@ -498,10 +498,16 @@ def __arrow_c_stream__(self: Self, requested_schema: object | None = None) -> ob | |
| pa_table = self.to_arrow() | ||
| return pa_table.__arrow_c_stream__(requested_schema=requested_schema) | ||
|
|
||
| def lazy(self: Self) -> LazyFrame[Any]: | ||
| def lazy(self: Self, *, backend: Implementation | None = None) -> LazyFrame[Any]: | ||
| """Lazify the DataFrame (if possible). | ||
|
|
||
| If a library does not support lazy execution, then this is a no-op. | ||
| If `backend` is specified, then a conversion between different backends | ||
| might be triggered. | ||
| If a library does not support lazy execution and `backend` is not specified, | ||
| then this is a no-op. | ||
|
|
||
| Arguments: | ||
| backend: specifies which Implementation to convert to. | ||
|
|
||
| Returns: | ||
| A new LazyFrame. | ||
|
|
@@ -545,7 +551,21 @@ def lazy(self: Self) -> LazyFrame[Any]: | |
| bar: [[6,7,8]] | ||
| ham: [["a","b","c"]] | ||
| """ | ||
| return self._lazyframe(self._compliant_frame.lazy(), level="lazy") | ||
| supported_backends = ( | ||
| Implementation.DASK, | ||
| Implementation.DUCKDB, | ||
| Implementation.POLARS, | ||
| ) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. RIP Pyspark π Happy to add it later no worries, I am just kidding!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I will try to look into adding PySpark in a follow-up PR |
||
| if backend is not None and backend not in supported_backends: | ||
| msg = ( | ||
| "Not supported backend" | ||
| f"\n\nExpected one of {supported_backends}, got {backend}" | ||
| ) | ||
| raise ValueError(msg) | ||
| return self._lazyframe( | ||
| self._compliant_frame.lazy(backend=backend), | ||
| level="lazy", | ||
| ) | ||
|
|
||
| def to_native(self: Self) -> DataFrameT: | ||
| """Convert Narwhals DataFrame to native one. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.