-
Notifications
You must be signed in to change notification settings - Fork 176
feat(expr-ir): Impl DataFrame.unique
#3364
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
Child of #2572, core `LogicalPlan` node
| class todo: # pragma: no cover # noqa: N801 | ||
| """A variation of `not_implemented`, for shorter-lived placeholders.""" |
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.
I still want to have this, but it doesn't make sense now that it isn't used in the PR 😅
| pytest.param( | ||
| ncs.first().last(), | ||
| InvalidOperationError, | ||
| # TODO @dangotbanned: Fix this in another PR | ||
| # Need to be stricter on the Selector check | ||
| marks=pytest.mark.xfail( | ||
| reason="narwhals/_plan/_expansion.py:160: 'Last' object has no attribute 'iter_expand_names'", | ||
| raises=AttributeError, | ||
| ), | ||
| ), |
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.
Follow-up PR
Be stricter here.
Calling an Expr method on a Selector preserves the runtime type of Selector
narwhals/narwhals/_plan/_parse.py
Lines 147 to 161 in 9b9122b
| def _parse_into_selector( | |
| input: ColumnNameOrSelector | Expr, /, *, require_all: bool = True | |
| ) -> Selector: | |
| if is_selector(input): | |
| selector = input | |
| elif isinstance(input, str): | |
| import narwhals._plan.selectors as cs | |
| selector = cs.by_name(input, require_all=require_all) | |
| elif is_expr(input): | |
| selector = input.meta.as_selector() | |
| else: | |
| msg = f"cannot turn {qualified_type_name(input)!r} into a selector" | |
| raise TypeError(msg) | |
| return selector |
Some related bits
narwhals/narwhals/_plan/meta.py
Lines 83 to 88 in 9b9122b
| def as_selector(self) -> Selector: | |
| """Try to turn this expression into a selector. | |
| Raises if the underlying expressions is not a column or selector. | |
| """ | |
| return self._ir.to_selector_ir().to_narwhals() |
narwhals/narwhals/_plan/_guards.py
Lines 97 to 101 in 9b9122b
| def is_column_name_or_selector( | |
| obj: Any, *, allow_expr: bool = False | |
| ) -> TypeIs[ColumnNameOrSelector]: | |
| tps = (str, _selectors().Selector) if not allow_expr else (str, _expr().Expr) | |
| return isinstance(obj, tps) |
A failure on these will now give a pretty granular id that points to the issue
Description
This'll be the first in a series of PRs that close gaps in non-
Exprfeatures.Will be focusing on what is defined in
DslPlan, and this one is forDslPlan::Distinct.Related issues
ExprIR #2572