Skip to content

feat(expr-ir): Add list.sort #3359

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.

Sign up for GitHub

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

Jump to bottom
Merged
dangotbanned merged 16 commits into from
Dec 18, 2025
Merged

feat(expr-ir): Add list.sort #3359

dangotbanned merged 16 commits into from
Dec 18, 2025
+223 −13

Conversation

@dangotbanned
Copy link
Member

@dangotbanned dangotbanned commented Dec 17, 2025 •
edited
Loading

Description

I mentioned in (#3356 (comment)) that I had a solution for this one.

Related issues

@dangotbanned dangotbanned mentioned this pull request Dec 17, 2025
Open
10 tasks
@dangotbanned dangotbanned added internal pyarrow Issue is related to pyarrow backend labels Dec 17, 2025
dangotbanned
dangotbanned commented Dec 17, 2025
View reviewed changes
dangotbanned
dangotbanned commented Dec 17, 2025
View reviewed changes
@dangotbanned
refactor: Kinda simplify list_sort
c37baff 
#3359 (comment), #3359 (comment)
@dangotbanned
fix(typing): Use narrower NonListTypeT
73eb3dc 
- Most `list.*` methods won't work for nested lists
- Making this easier to spot next time by keeping `implode/explode` together
@dangotbanned
Give sort_indices a facelift
451606a 
- Renamed `order_by` -> `by`
- More clearly separate tabular/array input
  - `pyarrow` accepts keys when sorting arrays
  - But they are noops, so removing that here
- Add a docstring
  - Mainly for the precedence part
dangotbanned
dangotbanned commented Dec 18, 2025
View reviewed changes
@dangotbanned dangotbanned marked this pull request as ready for review December 18, 2025 15:38
@dangotbanned dangotbanned added the documentation Improvements or additions to documentation label Dec 18, 2025
dangotbanned
dangotbanned commented Dec 18, 2025
View reviewed changes
narwhals/_plan/options.py
Comment on lines +167 to +169
def _ensure_single_nulls_last(self, backend: Backend, /) -> bool:
return self.nulls_last

Copy link
Member Author

@dangotbanned dangotbanned Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of a hack to avoid a runtime check between SortOptions and SortMultipleOptions in sort_indices

narwhals/narwhals/_plan/options.py

Lines 190 to 193 in 451606a

def _ensure_single_nulls_last(self, backend: Backend, /) -> bool:
first = self.nulls_last[0]
if len(self.nulls_last) != 1 and any(x != first for x in self.nulls_last[1:]):
msg = f"{Implementation.from_backend(backend)!r} does not support multiple values for `nulls_last`, got: {self.nulls_last!r}" # pragma: no cover

@dangotbanned dangotbanned mentioned this pull request Dec 18, 2025
Draft
@dangotbanned dangotbanned merged commit 9b9122b into oh-nodes Dec 18, 2025
38 of 43 checks passed
@dangotbanned dangotbanned deleted the expr-ir/list-sort-2 branch December 18, 2025 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation internal pyarrow Issue is related to pyarrow backend

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dangotbanned