-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ENH: support kind and na_position kwargs in Series.sort_index #14445
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 5 commits
d1d6513
aae1c1c
43dd994
b013103
423c771
05e9e52
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 |
---|---|---|
|
@@ -1773,7 +1773,7 @@ def _try_kind_sort(arr): | |
|
||
@Appender(generic._shared_docs['sort_index'] % _shared_doc_kwargs) | ||
def sort_index(self, axis=0, level=None, ascending=True, inplace=False, | ||
sort_remaining=True): | ||
kind='quicksort', na_position='last', sort_remaining=True): | ||
|
||
axis = self._get_axis_number(axis) | ||
index = self.index | ||
|
@@ -1782,12 +1782,16 @@ def sort_index(self, axis=0, level=None, ascending=True, inplace=False, | |
sort_remaining=sort_remaining) | ||
elif isinstance(index, MultiIndex): | ||
from pandas.core.groupby import _lexsort_indexer | ||
indexer = _lexsort_indexer(index.labels, orders=ascending) | ||
indexer = _ensure_platform_int(indexer) | ||
new_index = index.take(indexer) | ||
indexer = _lexsort_indexer(index.labels, orders=ascending, | ||
na_position=na_position) | ||
|
||
else: | ||
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. can you put:
outside the if-else block 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. Yes; moved. |
||
new_index, indexer = index.sort_values(return_indexer=True, | ||
ascending=ascending) | ||
from pandas.core.groupby import _nargsort | ||
indexer = _nargsort(index, kind=kind, ascending=ascending, | ||
na_position=na_position) | ||
|
||
indexer = _ensure_platform_int(indexer) | ||
new_index = index.take(indexer) | ||
|
||
new_values = self._values.take(indexer) | ||
result = self._constructor(new_values, index=new_index) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,3 +144,28 @@ def test_sort_index_multiindex(self): | |
# rows share same level='A': sort has no effect without remaining lvls | ||
res = s.sort_index(level='A', sort_remaining=False) | ||
assert_series_equal(s, res) | ||
|
||
def test_sort_index_kind(self): | ||
# GH #14444 & #13589: Add support for sort algo choosing | ||
series = Series(index=[3, 2, 1, 4, 3]) | ||
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. add a comment with the github reference 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. Added |
||
expected_series = Series(index=[1, 2, 3, 3, 4]) | ||
|
||
index_sorted_series = series.sort_index(kind='mergesort') | ||
assert_series_equal(expected_series, index_sorted_series) | ||
|
||
index_sorted_series = series.sort_index(kind='quicksort') | ||
assert_series_equal(expected_series, index_sorted_series) | ||
|
||
index_sorted_series = series.sort_index(kind='heapsort') | ||
assert_series_equal(expected_series, index_sorted_series) | ||
|
||
def test_sort_index_na_position(self): | ||
series = Series(index=[3, 2, 1, 4, 3, np.nan]) | ||
|
||
expected_series_first = Series(index=[np.nan, 1, 2, 3, 3, 4]) | ||
index_sorted_series = series.sort_index(na_position='first') | ||
assert_series_equal(expected_series_first, index_sorted_series) | ||
|
||
expected_series_last = Series(index=[1, 2, 3, 3, 4, np.nan]) | ||
index_sorted_series = series.sort_index(na_position='last') | ||
assert_series_equal(expected_series_last, index_sorted_series) |
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.
Can you leave this line out? (if it's not supported, not really needed to pass it)
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.
To be clear, leave out the
na_position
argument from_lexsort_indexer
?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.
yes (as it has no effect I understood?)
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.
Yes, that is true. Unfortunately, it has no effect right now. I pushed that change now.