-
Notifications
You must be signed in to change notification settings - Fork 190
feat: Allow nested structures in lit
#3424
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
ff8e647
db18590
c49ccbb
a897470
3697d12
d66d4e6
e3cee73
9b615c5
fc8ef14
8cf634b
1eb8451
69abb91
dd5c623
04825bf
9f65f3a
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 |
|---|---|---|
|
|
@@ -114,9 +114,15 @@ def func(cols: Iterable[ir.Value]) -> ir.Value: | |
|
|
||
| return self._expr._from_elementwise_horizontal_op(func, *exprs) | ||
|
|
||
| def lit(self, value: Any, dtype: IntoDType | None) -> IbisExpr: | ||
| def lit(self, value: PythonLiteral, dtype: IntoDType | None) -> IbisExpr: | ||
|
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. Oh nice, no more |
||
| def func(_df: IbisLazyFrame) -> Sequence[ir.Value]: | ||
| if isinstance(value, dict) and len(value) == 0: | ||
| msg = "Cannot create an empty struct type for Ibis backend" | ||
| raise NotImplementedError(msg) | ||
|
|
||
| ibis_dtype = narwhals_to_native_dtype(dtype, self._version) if dtype else None | ||
| if isinstance(value, dict): | ||
| return [ibis.struct(value, type=ibis_dtype)] | ||
| return [lit(value, ibis_dtype)] | ||
|
FBruzzesi marked this conversation as resolved.
Outdated
|
||
|
|
||
| return self._expr( | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,7 +25,7 @@ | |||||||||||||||
| from typing_extensions import TypeAlias | ||||||||||||||||
|
|
||||||||||||||||
| from narwhals._utils import Implementation, Version | ||||||||||||||||
| from narwhals.typing import IntoDType, NonNestedLiteral | ||||||||||||||||
| from narwhals.typing import IntoDType, PythonLiteral | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| Incomplete: TypeAlias = Any | ||||||||||||||||
|
|
@@ -83,17 +83,46 @@ def func(df: PandasLikeDataFrame) -> list[PandasLikeSeries]: | |||||||||||||||
| context=self, | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| def lit(self, value: NonNestedLiteral, dtype: IntoDType | None) -> PandasLikeExpr: | ||||||||||||||||
| def lit(self, value: PythonLiteral, dtype: IntoDType | None) -> PandasLikeExpr: | ||||||||||||||||
| def _lit_pandas_series(df: PandasLikeDataFrame) -> PandasLikeSeries: | ||||||||||||||||
| pandas_series = self._series.from_iterable( | ||||||||||||||||
| if isinstance(value, (list, tuple, dict)): | ||||||||||||||||
| try: | ||||||||||||||||
| import pandas as pd # ignore-banned-import | ||||||||||||||||
| import pyarrow as pa # ignore-banned-import | ||||||||||||||||
| except ImportError as exc: # pragma: no cover | ||||||||||||||||
| msg = ( | ||||||||||||||||
| "Nested structures require pyarrow to be installed for pandas backend. " | ||||||||||||||||
| "Please install pyarrow: pip install pyarrow" | ||||||||||||||||
| ) | ||||||||||||||||
| raise ImportError(msg) from exc | ||||||||||||||||
|
|
||||||||||||||||
| from narwhals._arrow.utils import ( | ||||||||||||||||
| narwhals_to_native_dtype as _to_arrow_dtype, | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| array_value = list(value) if isinstance(value, tuple) else value | ||||||||||||||||
| pa_dtype = _to_arrow_dtype(dtype, self._version) if dtype else None | ||||||||||||||||
| pa_array = pa.array([array_value], type=pa_dtype) # type: ignore[arg-type, list-item] | ||||||||||||||||
|
|
||||||||||||||||
| # Use ArrowExtensionArray to avoid pandas unpacking the nested structure | ||||||||||||||||
| ns = self._implementation.to_native_namespace() | ||||||||||||||||
| pandas_series_native = ns.Series( | ||||||||||||||||
| pd.arrays.ArrowExtensionArray(pa_array), # type: ignore[attr-defined] | ||||||||||||||||
| name="literal", | ||||||||||||||||
| index=df._native_frame.index[0:1], | ||||||||||||||||
| ) | ||||||||||||||||
|
Comment on lines
+107
to
+113
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 the wrapping part of (#3424 (comment)) should be reused here. Maybe The two more useful parts IMO are:
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. Okay yeah so what I'm thinking is a new constructor (or two?) on All of these are parts of a constructor for both native & compliant, and they're spread across 4 modules. narwhals/narwhals/_pandas_like/dataframe.py Line 308 in dd5c623
narwhals/narwhals/_pandas_like/namespace.py Lines 86 to 87 in dd5c623
narwhals/narwhals/_pandas_like/series.py Lines 204 to 206 in dd5c623
narwhals/narwhals/_pandas_like/utils.py Line 668 in dd5c623
|
||||||||||||||||
|
|
||||||||||||||||
| return self._series.from_native(pandas_series_native, context=self) | ||||||||||||||||
|
|
||||||||||||||||
| pandas_like_series = self._series.from_iterable( | ||||||||||||||||
| data=[value], | ||||||||||||||||
| name="literal", | ||||||||||||||||
| index=df._native_frame.index[0:1], | ||||||||||||||||
| context=self, | ||||||||||||||||
| ) | ||||||||||||||||
| if dtype: | ||||||||||||||||
| return pandas_series.cast(dtype) | ||||||||||||||||
| return pandas_series | ||||||||||||||||
| return pandas_like_series.cast(dtype) | ||||||||||||||||
| return pandas_like_series | ||||||||||||||||
|
|
||||||||||||||||
| return PandasLikeExpr( | ||||||||||||||||
| lambda df: [_lit_pandas_series(df)], | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, isn't this a nice diff π