Skip to content

API: Change default for Index.union sort #25007

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

Closed
wants to merge 14 commits into from
Closed

API: Change default for Index.union sort #25007

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
aac172c
[WIP]: API: Change default for Index.union sort
TomAugspurger Jan 29, 2019
d4bcc55
update test
TomAugspurger Jan 29, 2019
45c827c
fixups
TomAugspurger Jan 30, 2019
68b72a6
Merge remote-tracking branch 'upstream/master' into 24959-union-default
TomAugspurger Jan 30, 2019
8716f97
multi
TomAugspurger Jan 30, 2019
f7056d5
doc typo
TomAugspurger Jan 30, 2019
e82cbb1
intersection
TomAugspurger Jan 30, 2019
2a2de25
whatsnew
TomAugspurger Jan 30, 2019
5c3da74
update whatsnew
TomAugspurger Jan 30, 2019
ce6d1db
Merge remote-tracking branch 'upstream/master' into 24959-union-default
TomAugspurger Jan 31, 2019
52a2f2f
symdiff
TomAugspurger Jan 31, 2019
bb848f1
whatsnew
TomAugspurger Jan 31, 2019
b15dc7e
doc
TomAugspurger Jan 31, 2019
27b5b16
index multi
TomAugspurger Jan 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
update test
  • Loading branch information
@TomAugspurger
TomAugspurger committed Jan 29, 2019
commit d4bcc55a161c56f485b95fdbcfcff88344df47d4
2 changes: 1 addition & 1 deletion pandas/core/indexes/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2268,7 +2268,7 @@ def union(self, other, sort=None):

.. versionadded:: 0.24.0

.. versionchanged:: 0.24.0
.. versionchanged:: 0.24.1

Changed the default `sort` to None, matching the
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this being changed? this is certainly not a regression at all. This was the default behavior.

Copy link
Member

Choose a reason for hiding this comment

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

To be clear: no behaviour is changed. It was indeed the default, it stays the default. It's only the value that encodes the default that is changed (True -> None), so that True can mean something else (=always sort).

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, maybe it should be more clear in the doc-string

behavior of pandas 0.23.4 and earlier.
Expand Down
9 changes: 5 additions & 4 deletions pandas/tests/indexes/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,19 +856,20 @@ def test_union_from_iterables(self, klass, sort):
tm.assert_index_equal(result, everything.sort_values())
assert tm.equalContents(result, everything)

@pytest.mark.parametrize("sort", [True, False])
@pytest.mark.parametrize("sort", [None, True, False])
def test_union_identity(self, sort):
# TODO: replace with fixturesult
first = self.strIndex[5:20]

union = first.union(first, sort=sort)
assert union is first
# i.e. identity is not preserved when sort is True
assert (union is first) is (not sort)

union = first.union([], sort=sort)
assert union is first
assert (union is first) is (not sort)

union = Index([]).union(first, sort=sort)
assert union is first
assert (union is first) is (not sort)

@pytest.mark.parametrize("first_list", [list('ba'), list()])
@pytest.mark.parametrize("second_list", [list('ab'), list()])
Expand Down