-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
ENH: Add ignore_index to sort_index #30578
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 7 commits
7e461a1
1314059
8bcb313
2f36225
c03ed13
4a9c645
7a2651e
85cb197
15ab36c
bb6fee4
d4f787a
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 |
---|---|---|
|
@@ -229,3 +229,84 @@ def test_sort_index_intervalindex(self): | |
) | ||
result = result.columns.levels[1].categories | ||
tm.assert_index_equal(result, expected) | ||
|
||
@pytest.mark.parametrize( | ||
"original_dict, sorted_dict, ascending, ignore_index, output_index", | ||
[ | ||
({"A": [1, 2, 3]}, {"A": [3, 2, 1]}, False, True, [0, 1, 2]), | ||
({"A": [1, 2, 3]}, {"A": [1, 2, 3]}, True, True, [0, 1, 2]), | ||
({"A": [1, 2, 3]}, {"A": [3, 2, 1]}, False, False, [2, 1, 0]), | ||
({"A": [1, 2, 3]}, {"A": [1, 2, 3]}, True, False, [0, 1, 2]), | ||
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. Minor but can you use a non-default index as part of the expectation? Right now the parametrization can't disambiguate between the default index and 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. thanks! changed! |
||
], | ||
) | ||
def test_sort_index_ignore_index( | ||
self, original_dict, sorted_dict, ascending, ignore_index, output_index | ||
): | ||
# GH 30114 | ||
df = DataFrame(original_dict) | ||
expected_df = DataFrame(sorted_dict, index=output_index) | ||
|
||
sorted_df = df.sort_index(ascending=ascending, ignore_index=ignore_index) | ||
tm.assert_frame_equal(sorted_df, expected_df) | ||
tm.assert_frame_equal(df, DataFrame(original_dict)) | ||
|
||
# Test when inplace is True | ||
copied_df = df.copy() | ||
copied_df.sort_index( | ||
ascending=ascending, ignore_index=ignore_index, inplace=True | ||
) | ||
tm.assert_frame_equal(copied_df, expected_df) | ||
tm.assert_frame_equal(df, DataFrame(original_dict)) | ||
|
||
@pytest.mark.parametrize( | ||
"original_dict, sorted_dict, ascending, ignore_index, output_index", | ||
[ | ||
( | ||
{"M1": [1, 2], "M2": [3, 4]}, | ||
{"M1": [1, 2], "M2": [3, 4]}, | ||
True, | ||
True, | ||
[0, 1], | ||
), | ||
( | ||
{"M1": [1, 2], "M2": [3, 4]}, | ||
{"M1": [2, 1], "M2": [4, 3]}, | ||
False, | ||
True, | ||
[0, 1], | ||
), | ||
( | ||
{"M1": [1, 2], "M2": [3, 4]}, | ||
{"M1": [1, 2], "M2": [3, 4]}, | ||
True, | ||
False, | ||
MultiIndex.from_tuples([[2, 1], [3, 4]], names=list("AB")), | ||
), | ||
( | ||
{"M1": [1, 2], "M2": [3, 4]}, | ||
{"M1": [2, 1], "M2": [4, 3]}, | ||
False, | ||
False, | ||
MultiIndex.from_tuples([[3, 4], [2, 1]], names=list("AB")), | ||
), | ||
], | ||
) | ||
def test_sort_index_ignore_index_multi_index( | ||
self, original_dict, sorted_dict, ascending, ignore_index, output_index | ||
): | ||
# GH 30114, this is to test ignore_index on MulitIndex of index | ||
mi = MultiIndex.from_tuples([[2, 1], [3, 4]], names=list("AB")) | ||
df = DataFrame(original_dict, index=mi) | ||
expected_df = DataFrame(sorted_dict, index=output_index) | ||
|
||
sorted_df = df.sort_index(ascending=ascending, ignore_index=ignore_index) | ||
tm.assert_frame_equal(sorted_df, expected_df) | ||
tm.assert_frame_equal(df, DataFrame(original_dict, index=mi)) | ||
|
||
# Test when inplace is True | ||
copied_df = df.copy() | ||
copied_df.sort_index( | ||
ascending=ascending, ignore_index=ignore_index, inplace=True | ||
) | ||
tm.assert_frame_equal(copied_df, expected_df) | ||
tm.assert_frame_equal(df, DataFrame(original_dict, index=mi)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -135,3 +135,35 @@ def test_sort_index_intervals(self): | |
[3, 2, 1, np.nan], IntervalIndex.from_arrays([3, 2, 1, 0], [4, 3, 2, 1]) | ||
) | ||
tm.assert_series_equal(result, expected) | ||
|
||
@pytest.mark.parametrize( | ||
"original_list, sorted_list, ascending, ignore_index, output_index", | ||
[ | ||
([2, 3, 6, 1], [2, 3, 6, 1], True, True, [0, 1, 2, 3]), | ||
([2, 3, 6, 1], [2, 3, 6, 1], True, False, [0, 1, 2, 3]), | ||
([2, 3, 6, 1], [1, 6, 3, 2], False, True, [0, 1, 2, 3]), | ||
([2, 3, 6, 1], [1, 6, 3, 2], False, False, [3, 2, 1, 0]), | ||
], | ||
) | ||
def test_sort_index_ignore_index( | ||
self, original_list, sorted_list, ascending, ignore_index, output_index | ||
): | ||
# GH 30114 | ||
# GH 30114 | ||
simonjayhawkins marked this conversation as resolved.
Show resolved
Hide resolved
|
||
sr = Series(original_list) | ||
simonjayhawkins marked this conversation as resolved.
Show resolved
Hide resolved
|
||
expected = Series(sorted_list, index=output_index) | ||
|
||
# Test when inplace is False | ||
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 inplace be parameterised easily 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. emm, i think in this case, if using parameterization, might not look very clean and less readable 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. I think this should be parametrized - what do you think makes it less readable? 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. ok with trying parameterize as a followup, as for .sort_values and .duplicated these tests look very much like this. 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. emm, okay! i thought with Sorry to bring up discussion here, it definitely can be parametrized, was just my preference on such case. I will parametrize all three methods in the follow-up PR for sure! |
||
sorted_sr = sr.sort_index(ascending=ascending, ignore_index=ignore_index) | ||
tm.assert_series_equal(sorted_sr, expected) | ||
|
||
tm.assert_series_equal(sr, Series(original_list)) | ||
|
||
# Test when inplace is True | ||
copied_sr = sr.copy() | ||
copied_sr.sort_index( | ||
ascending=ascending, ignore_index=ignore_index, inplace=True | ||
) | ||
tm.assert_series_equal(copied_sr, expected) | ||
|
||
tm.assert_series_equal(sr, Series(original_list)) |
Uh oh!
There was an error while loading. Please reload this page.