Skip to content

BUG: Fix duplicates in intersection of multiindexes #36927

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

Merged
merged 27 commits into from
Nov 29, 2020
Merged

BUG: Fix duplicates in intersection of multiindexes #36927

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
cdefaae
Fix duplicates in intersectin of multiindexes
phofl Oct 6, 2020
fbd63f2
Fix duplicates in index intersection
phofl Oct 6, 2020
53a37d1
Modify test and avoid None issues
phofl Oct 6, 2020
5675a4e
Fix failing test
phofl Oct 6, 2020
134936c
Merge branch 'master' of https://github.com/pandas-dev/pandas into 36915
phofl Oct 9, 2020
582c0b9
Change comment
phofl Oct 9, 2020
7805de5
Add unique after intersection
phofl Oct 11, 2020
67691df
Merge branch 'master' of https://github.com/pandas-dev/pandas into 36915
phofl Oct 11, 2020
8fb0055
Merge branch '31326' into 36915
phofl Oct 11, 2020
66b519f
Fix merge bug
phofl Oct 11, 2020
cb1477b
Add tests and whatsnew
phofl Oct 11, 2020
0fb2561
Add rename
phofl Oct 26, 2020
3c19d57
Merge branch 'master' of https://github.com/pandas-dev/pandas into 36915
phofl Oct 26, 2020
10524fd
Fix check in merge operation
phofl Oct 26, 2020
3dde0ee
Exit set ops when nonunique
phofl Nov 12, 2020
a0a1a33
Merge branch 'master' of https://github.com/pandas-dev/pandas into 36915
phofl Nov 12, 2020
45dfb84
Merge branch 'master' of https://github.com/pandas-dev/pandas into 36915
phofl Nov 22, 2020
d71a499
Roll back to initial version
phofl Nov 22, 2020
d873d5a
Change whatsnew
phofl Nov 22, 2020
e90239a
Move whatsnew
phofl Nov 27, 2020
c2b448a
Merge branch 'master' of https://github.com/pandas-dev/pandas into 36915
phofl Nov 27, 2020
742716e
Change gh reference
phofl Nov 27, 2020
321797a
Remove pd
phofl Nov 27, 2020
a980ec0
Remove whatsnew from 1.2
phofl Nov 28, 2020
972fd48
Fix test
phofl Nov 28, 2020
fe1ded4
Make condition more clear and add assert
phofl Nov 28, 2020
8e4d47b
Use shape for equality check
phofl Nov 28, 2020
File filter

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
Merge branch 'master' of https://github.com/pandas-dev/pandas into 36915 
� Conflicts: �	doc/source/whatsnew/v1.2.0.rst �	pandas/tests/indexes/test_setops.py
  • Loading branch information
@phofl
phofl committed Nov 12, 2020
commit a0a1a33daefafbce7d747f28a2098f74a91a19f1
3 changes: 2 additions & 1 deletion doc/source/whatsnew/v1.2.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,9 @@ MultiIndex

- Bug in :meth:`DataFrame.xs` when used with :class:`IndexSlice` raises ``TypeError`` with message ``"Expected label or tuple of labels"`` (:issue:`35301`)
- Bug in :meth:`DataFrame.reset_index` with ``NaT`` values in index raises ``ValueError`` with message ``"cannot convert float NaN to integer"`` (:issue:`36541`)
- Bug in :meth:`DataFrame.combine_first` when used with :class:`MultiIndex` containing string and ``NaN`` values raises ``TypeError`` (:issue:`36562`)
- Bug in :meth:`MultiIndex.intersection` returned duplicates when at least one of the indexes had duplicates (:issue:`36915`)
Copy link
Member

@simonjayhawkins simonjayhawkins Nov 27, 2020

Choose a reason for hiding this comment

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

xref #36915 (comment) This is fixing a regression from 1.0.5? Can we target 1.1.5 with this?
Copy link
Member Author

@phofl phofl Nov 27, 2020

Choose a reason for hiding this comment

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

Moved the whatsnew, but this may be tricky to backport.
Copy link
Member

@simonjayhawkins simonjayhawkins Nov 27, 2020

Choose a reason for hiding this comment

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

I've not yet reviewed this PR. are there any changes other than those required to fix the regression here?
Copy link
Member Author

@phofl phofl Nov 27, 2020

Choose a reason for hiding this comment

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

Merge and setops relied partially on the wrong behavior, had to fix this too.
Copy link
Member

@simonjayhawkins simonjayhawkins Nov 28, 2020

Choose a reason for hiding this comment

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

hmm, not investigated fully, but this PR may sit on top of #37171 (which incidentally caused a perf regression #37171 (comment) so maybe that we want to revert that first before committing the changes here anyhow)
Copy link
Member

@simonjayhawkins simonjayhawkins Nov 28, 2020

Choose a reason for hiding this comment

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

but this may be tricky to backport.

opened #38133 to test
Copy link
Member

@simonjayhawkins simonjayhawkins Nov 28, 2020

Choose a reason for hiding this comment

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

not checked all the envs, but a quick check on a couple shows the same tests failing on the backport as here. so that's encouraging.

I think we should target as a backport. and if issues down the road will just have to move the release note back.
Copy link
Member Author

@phofl phofl Nov 28, 2020

Choose a reason for hiding this comment

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

Yep, thats a good sign. Lets try this then, when everything is green here


I/O
^^^

Expand Down Expand Up @@ -584,6 +584,7 @@ Other
- Bug in :meth:`Index.union` behaving differently depending on whether operand is a :class:`Index` or other list-like (:issue:`36384`)
- Passing an array with 2 or more dimensions to the :class:`Series` constructor now raises the more specific ``ValueError``, from a bare ``Exception`` previously (:issue:`35744`)
- Bug in ``accessor.DirNamesMixin``, where ``dir(obj)`` wouldn't show attributes defined on the instance (:issue:`37173`).
- Bug in :meth:`Series.nunique` with ``dropna=True`` was returning incorrect results when both ``NA`` and ``None`` missing values were present (:issue:`37566`)
- Bug in :meth:`Index.intersection` returned duplicates when at least one of the indexes had duplicates (:issue:`31326`)

.. ---------------------------------------------------------------------------
Expand Down
13 changes: 13 additions & 0 deletions pandas/tests/indexes/test_setops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,19 @@ def test_union_dtypes(left, right, expected):
assert result == expected


def test_dunder_inplace_setops_deprecated(index):
# GH#37374 these will become logical ops, not setops

with tm.assert_produces_warning(FutureWarning):
index |= index

with tm.assert_produces_warning(FutureWarning):
index &= index

with tm.assert_produces_warning(FutureWarning):
index ^= index


@pytest.mark.parametrize("values", [[1, 2, 2, 3], [3, 3]])
def test_intersection_duplicates(values):
# GH: 31326
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.