Skip to content

Implement sort_remaining for sort_index - #14033

Merged
rapids-bot[bot] merged 3 commits into
NVIDIA:branch-23.10from
wence-:wence/fea/sort-index-sort-remaining
Sep 5, 2023
Merged

Implement sort_remaining for sort_index#14033
rapids-bot[bot] merged 3 commits into
NVIDIA:branch-23.10from
wence-:wence/fea/sort-index-sort-remaining

Conversation

@wence-

@wence- wence- commented Sep 4, 2023

Copy link
Copy Markdown
Contributor

Description

Previously, the sort_remaining argument to sort_index was ignored. Passing sort_remaining=False would raise a NotImplementedError. Moreover, for a multiindex, sort_remaining=True was not handled correctly: if not all levels were requested as sorted, sort_index would behave as if sort_remaining=False had been passed.

To fix this case, construct the sort order based on first the provided levels and, if sort_remaining=True, the left-over levels (in index order).

To facilitate this, refactor the internal _get_columns_by_label function to always return a Frame-like object (previously, if we had a Frame we would get back a ColumnAccessor, and it was only for IndexedFrame and above that we'd get something of Self-like type back). This meant that calling _get_sorted_inds with by != None was not possible on an Index or MultiIndex (the code assumed we'd get a Frame back).

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@wence-
wence- requested a review from a team as a code owner September 4, 2023 14:50
@wence-
wence- requested review from mroeschke and vyasr September 4, 2023 14:50
@github-actions github-actions Bot added the Python Affects Python cuDF API. label Sep 4, 2023
@wence- wence- added bug Something isn't working non-breaking Non-breaking change and removed Python Affects Python cuDF API. labels Sep 4, 2023

@galipremsagar galipremsagar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, minor comment.

Comment thread python/cudf/cudf/tests/test_multiindex.py
@wence-

wence- commented Sep 5, 2023

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 0b01fe4 into NVIDIA:branch-23.10 Sep 5, 2023
@wence-
wence- deleted the wence/fea/sort-index-sort-remaining branch September 6, 2023 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEA] Allow DataFrame.sort_index(level=n) to tiebreak on lexsortedness in mode.pandas_compatable

4 participants