-
Notifications
You must be signed in to change notification settings - Fork 170
chore(typing): Fill out CompliantNamespace protocol
#2202
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
Conversation
- Only needs to be the extra stuff - `_create_compliant_series` is removed in #2196
| *, | ||
| how: Literal["horizontal", "vertical", "diagonal"], | ||
| ) -> CompliantFrameT: ... | ||
| def when(self, predicate: CompliantExprT) -> Any: ... |
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.
- Return type would require another generic protocol for
When - Doing that is a pretty low priority for now
narwhals/_spark_like/namespace.py
Outdated
| @property | ||
| def _expr(self) -> type[SparkLikeExpr]: | ||
| return SparkLikeExpr | ||
|
|
||
| def all(self: Self) -> SparkLikeExpr: | ||
| return SparkLikeExpr.from_column_names( | ||
| get_column_names, |
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.
Oooh we might be able to implement this (and maybe a few other methods) higher up in the protocol 🤯
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.
from narwhals.utils import get_column_names
class CompliantNamespace(Protocol[CompliantFrameT, CompliantExprT]):
@property
def _expr(self) -> type[CompliantExprT]: ...
def all(self) -> CompliantExprT:
return self._expr.from_column_names(
get_column_names,
function_name="all",
implementation=self._implementation,
backend_version=self._backend_version,
version=self._version,
)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.
Already most of the way there in
narwhals/narwhals/_pandas_like/namespace.py
Lines 77 to 96 in d860054
| # --- selection --- | |
| def col(self: Self, *column_names: str) -> PandasLikeExpr: | |
| return self._expr.from_column_names( | |
| passthrough_column_names(column_names), function_name="col", context=self | |
| ) | |
| def exclude(self: Self, excluded_names: Container[str]) -> PandasLikeExpr: | |
| return self._expr.from_column_names( | |
| partial(exclude_column_names, names=excluded_names), | |
| function_name="exclude", | |
| context=self, | |
| ) | |
| def nth(self: Self, *column_indices: int) -> PandasLikeExpr: | |
| return self._expr.from_column_indices(*column_indices, context=self) | |
| def all(self: Self) -> PandasLikeExpr: | |
| return self._expr.from_column_names( | |
| get_column_names, function_name="all", context=self | |
| ) |
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.
@FBruzzesi ignore this if you're not fully back yet 🙂
Thought you might like to see this new possibility - following the recent spec-ing of the Compliant* protocols
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.
Went ahead with that in (5d609a7)
We now have default implementations for 4x CompliantNamespace methods 😁
All backends (besides `polars`) now share the same implementation #2202 (comment)
- `FrameT` isn't relevant to what is used - Only needed before as `CompliantNamespace` didn't expose an `ExprT`
| ) | ||
|
|
||
| def mean_horizontal(self: Self, *exprs: ArrowExpr) -> IntoArrowExpr: | ||
| def mean_horizontal(self: Self, *exprs: ArrowExpr) -> ArrowExpr: |
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.
This only got flagged after filling out the protocol.
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.
thanks @dangotbanned !

What type of PR is this? (check all applicable)
Related issues
CompliantSeries.from_numpy#2196CompliantExpr#2119Checklist
If you have comments or can explain your changes, please do so below
_create_compliant_seriesis removed in refactor: AddCompliantSeries.from_numpy#2196CompliantExpr