-
Notifications
You must be signed in to change notification settings - Fork 205
feat: Added struct namespace with field method.
#2146
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 8 commits
0d18f96
496cbd0
9592277
aaebbbe
273f331
2d95d3d
decba02
f89f7d3
e75da43
ef8b863
fa802a4
5e04a46
47345f5
9517b91
e8623e2
2d4ce6b
825d29e
6f867ef
e54663c
8fe86ae
37dc7fc
104d40d
9ead00a
9e2a24d
20ec4eb
ecfd180
6c5597b
99a78e3
105d3f9
d21bdc2
9db179a
865bedd
3ad7ae9
7e5d813
c5cf993
4d36295
8701e43
4ed080c
195088c
71df89d
ebe4d6b
59fb567
736599e
f71e677
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| # Expr.str | ||
|
|
||
| | Method | arrow | duckdb | pandas-like | polars | spark-like | | ||
| |--------------|--------------------|--------------------|--------------------|--------------------|--------------------| | ||
| | field | :white_check_mark: | :white_check_mark: | :white_check_mark: | :white_check_mark: | :white_check_mark: | | ||
|
osoucy marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| # Series.str | ||
|
osoucy marked this conversation as resolved.
Outdated
|
||
|
|
||
| | Method | arrow | pandas-like | polars | | ||
| |--------------|---------------------|--------------------|--------------------| | ||
| | field | :white_check_mark: | :white_check_mark: | :white_check_mark: | | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # `narwhals.Expr.struct` | ||
|
|
||
| ::: narwhals.expr.ExprStructNamespace | ||
| handler: python | ||
| options: | ||
| members: | ||
| - field | ||
| show_source: false | ||
| show_bases: false |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # `narwhals.Series.struct` | ||
|
|
||
| ::: narwhals.series.SeriesStructNamespace | ||
| handler: python | ||
| options: | ||
| members: | ||
| - field | ||
| show_source: false | ||
| show_bases: false |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,21 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| from typing import TYPE_CHECKING | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| from narwhals._expression_parsing import reuse_series_namespace_implementation | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if TYPE_CHECKING: | ||||||||||||||||||||||||||||||||||||||||||||||
| from typing_extensions import Self | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| from narwhals._arrow.expr import ArrowExpr | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| class ArrowExprStructNamespace: | ||||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self: Self, expr: ArrowExpr) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||
| self._compliant_expr = expr | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def field(self: Self, name: str) -> ArrowExpr: | ||||||||||||||||||||||||||||||||||||||||||||||
| self._compliant_expr._evaluate_output_names = lambda _col: [name] | ||||||||||||||||||||||||||||||||||||||||||||||
| return reuse_series_namespace_implementation( | ||||||||||||||||||||||||||||||||||||||||||||||
| self._compliant_expr, "struct", "field", name=name | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
|
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 feel like mutating the current expression might cause issues elsewhere. @osoucy was there a particular reason you chose this?
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. I had to use it becase "facilitator" methods such as The other option would be to not use facilitator methods and simply intantiate an expression / series class from scratch, but it would mean some code duplication.
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. Thanks for the detail @osoucy that was helpful I think that the pattern used here might be something to work backwards from It seems quite a bit more complex than what you might need, but hopefully you can see how it avoids mutating the current narwhals/narwhals/_pandas_like/expr_name.py Lines 63 to 84 in b986bfd
@FBruzzesi per #1876
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. Instead of creating another helper function (that might not get re-used), I modified the |
||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from typing import TYPE_CHECKING | ||
|
|
||
| import pyarrow.compute as pc | ||
|
|
||
| if TYPE_CHECKING: | ||
| from typing_extensions import Self | ||
|
|
||
| from narwhals._arrow.series import ArrowSeries | ||
|
|
||
|
|
||
| class ArrowSeriesStructNamespace: | ||
| def __init__(self: Self, series: ArrowSeries) -> None: | ||
| self._compliant_series: ArrowSeries = series | ||
|
|
||
| def field(self: Self, name: str) -> ArrowSeries: | ||
| self._compliant_series._name = name | ||
|
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. you can't mutate
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. Done.
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. For the They weren't available when you started the PR @osoucy
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. Some more context #2130 (comment)
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. this is fine as a follow-up
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. At least the current implementation avoids mutating the compliant series. Maybe I can look into @dangotbanned in a future PR?
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. All good @osoucy π
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. So we are good to merge? :) |
||
| return self._compliant_series._from_native_series( | ||
| pc.struct_field(self._compliant_series._native_series, name) | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from typing import TYPE_CHECKING | ||
|
|
||
| from duckdb import FunctionExpression | ||
|
|
||
| from narwhals._duckdb.utils import lit | ||
|
|
||
| if TYPE_CHECKING: | ||
| from typing_extensions import Self | ||
|
|
||
| from narwhals._duckdb.expr import DuckDBExpr | ||
|
|
||
|
|
||
| class DuckDBExprStructNamespace: | ||
| def __init__(self: Self, expr: DuckDBExpr) -> None: | ||
| self._compliant_expr = expr | ||
|
|
||
| def field(self: Self, name: str) -> DuckDBExpr: | ||
| self._compliant_expr._evaluate_output_names = lambda _col: [name] | ||
| return self._compliant_expr._from_call( | ||
| lambda _input: FunctionExpression("struct_extract", _input, lit(name)).alias( | ||
| name | ||
| ), | ||
| "field", | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from typing import TYPE_CHECKING | ||
|
|
||
| from narwhals._expression_parsing import reuse_series_namespace_implementation | ||
|
|
||
| if TYPE_CHECKING: | ||
| from typing_extensions import Self | ||
|
|
||
| from narwhals._pandas_like.expr import PandasLikeExpr | ||
|
|
||
|
|
||
| class PandasLikeExprStructNamespace: | ||
| def __init__(self: Self, expr: PandasLikeExpr) -> None: | ||
| self._compliant_expr = expr | ||
|
|
||
| def field(self, name: str) -> PandasLikeExpr: | ||
| self._compliant_expr._evaluate_output_names = lambda _col: [name] | ||
| return reuse_series_namespace_implementation( | ||
| self._compliant_expr, "struct", "field", name=name | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from typing import TYPE_CHECKING | ||
|
|
||
| if TYPE_CHECKING: | ||
| from typing_extensions import Self | ||
|
|
||
| from narwhals._pandas_like.series import PandasLikeSeries | ||
|
|
||
|
|
||
| class PandasLikeSeriesStructNamespace: | ||
| def __init__(self: Self, series: PandasLikeSeries) -> None: | ||
| self._compliant_series = series | ||
|
|
||
| def field(self: Self, name: str) -> PandasLikeSeries: | ||
| return self._compliant_series._from_native_series( | ||
| self._compliant_series._native_series.apply(lambda x: x[name]).rename(name), | ||
|
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. Instead of using apply can we use https://pandas.pydata.org/pandas-docs/dev/reference/api/pandas.Series.struct.field.html and only support this for pyarrow-backed dtypes?
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. I feel the majority of Pandas users don't leverage pyarrow-backed dtypes. I added a switch to check and use the |
||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from typing import TYPE_CHECKING | ||
|
|
||
| if TYPE_CHECKING: | ||
| from typing_extensions import Self | ||
|
|
||
| from narwhals._spark_like.expr import SparkLikeExpr | ||
|
|
||
|
|
||
| class SparkLikeExprStructNamespace: | ||
| def __init__(self: Self, expr: SparkLikeExpr) -> None: | ||
| self._compliant_expr = expr | ||
|
|
||
| def field(self: Self, name: str) -> SparkLikeExpr: | ||
| self._compliant_expr._evaluate_output_names = lambda _col: [name] | ||
| return self._compliant_expr._from_call(lambda col: col.getField(name), "field") |
Uh oh!
There was an error while loading. Please reload this page.