-
Notifications
You must be signed in to change notification settings - Fork 202
refactor: Organizing Compliant* APIs
#3045
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 15 commits
3bfefb0
bc6e890
1e8a770
a5f4432
bd3f691
8fa84e8
29a3038
2c31971
f727dcf
7eb1a27
4c24c9e
60706de
2525fc1
c67dd85
04dfd11
2464524
31fa908
f76b328
b20c1e1
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -81,6 +81,7 @@ def __ne__(self, value: Any, /) -> Self: ... # type: ignore[override] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class CompliantExpr( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CompliantColumn, Protocol[CompliantFrameT, CompliantSeriesOrNativeExprT_co] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # NOTE: `narwhals` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _implementation: Implementation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _evaluate_output_names: EvalNames[CompliantFrameT] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _alias_output_names: AliasNames | None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -92,16 +93,19 @@ def __call__( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __narwhals_expr__(self) -> None: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __narwhals_namespace__(self) -> CompliantNamespace[CompliantFrameT, Self]: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @classmethod | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def from_column_indices( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cls, *column_indices: int, context: _LimitedContext | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Self: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @classmethod | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def from_column_names( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cls, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| evaluate_column_names: EvalNames[CompliantFrameT], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| *, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| context: _LimitedContext, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Self: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
97
to
108
Member
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'm somewhat torn on these I like that all you need to do is implement these two:
And then you get these 4 for "free"
narwhals/narwhals/_compliant/namespace.py Lines 53 to 76 in 29a3038
What I don't like is that
Recently, this PR removed the requirement of also having I'd like to continue that trend and work towards removing I've been wanting to drop down to 1x
Which would just mean defining it on the class like e.g. narwhals/narwhals/_arrow/expr.py Lines 28 to 29 in 29a3038
If we do that, then:
Member
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. We could also then make That doesn't depend on
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.
sure, if that works, why not |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @classmethod | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def from_column_indices( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cls, *column_indices: int, context: _LimitedContext | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def broadcast( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self, kind: Literal[ExprKind.AGGREGATION, ExprKind.LITERAL] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Self: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _eval_names_indices(indices: Sequence[int], /) -> EvalNames[CompliantFrameT]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -111,6 +115,7 @@ def fn(df: CompliantFrameT) -> Sequence[str]: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return fn | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # NOTE: `polars` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def all(self) -> Self: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def any(self) -> Self: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def count(self) -> Self: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -137,35 +142,24 @@ def map_batches( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| *, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| returns_scalar: bool, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Self: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def broadcast( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self, kind: Literal[ExprKind.AGGREGATION, ExprKind.LITERAL] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Self: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _is_multi_output_unnamed(self) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Return `True` for multi-output aggregations without names. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| For example, column `'a'` only appears in the output as a grouping key: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| df.group_by('a').agg(nw.all().sum()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| It does not get included in: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @property | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def name(self) -> NameNamespace[Self]: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nw.all().sum(). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert self._metadata is not None # noqa: S101 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return self._metadata.expansion_kind.is_multi_unnamed() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _evaluate_aliases( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self: CompliantExpr[CompliantFrameT, Any], frame: CompliantFrameT, / | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Sequence[str]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| names = self._evaluate_output_names(frame) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class ImplExpr( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CompliantExpr[CompliantFrameT, CompliantSeriesOrNativeExprT_co], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Protocol[CompliantFrameT, CompliantSeriesOrNativeExprT_co], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _evaluate_aliases(self, frame: CompliantFrameT, /) -> Sequence[str]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # NOTE: Ignore intermittent [False Negative] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Argument of type "CompliantFrameT@ImplExpr" cannot be assigned to parameter of type "CompliantFrameT@ImplExpr" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Type "CompliantFrameT@ImplExpr" is not assignable to type "CompliantFrameT@ImplExpr" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| names = self._evaluate_output_names(frame) # pyright: ignore[reportArgumentType] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return alias(names) if (alias := self._alias_output_names) else names | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @property | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def name(self) -> NameNamespace[Self]: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class DepthTrackingExpr( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CompliantExpr[CompliantFrameT, CompliantSeriesOrNativeExprT_co], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ImplExpr[CompliantFrameT, CompliantSeriesOrNativeExprT_co], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Protocol[CompliantFrameT, CompliantSeriesOrNativeExprT_co], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _depth: int | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -888,8 +882,7 @@ def struct(self) -> EagerExprStructNamespace[Self]: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # mypy thinks `NativeExprT` should be covariant, pyright thinks it should be invariant | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class LazyExpr( # type: ignore[misc] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CompliantExpr[CompliantLazyFrameT, NativeExprT], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Protocol[CompliantLazyFrameT, NativeExprT], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ImplExpr[CompliantLazyFrameT, NativeExprT], Protocol[CompliantLazyFrameT, NativeExprT] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _with_alias_output_names(self, func: AliasNames | None, /) -> Self: ... | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def alias(self, name: str) -> Self: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.