Skip to content

BUG: pd.concat with identical key leads to multi-indexing error #46546

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 16 commits into from
Apr 5, 2022
Merged

BUG: pd.concat with identical key leads to multi-indexing error #46546

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c624df5
Update concat.py
GYHHAHA Mar 28, 2022
58b2bb4
Update v1.5.0.rst
GYHHAHA Mar 28, 2022
fc2cef9
Merge branch 'pandas-dev:main' into patch-1
GYHHAHA Mar 28, 2022
2e4317c
Update test_index.py
GYHHAHA Mar 28, 2022
ef8337a
Update test_index.py
GYHHAHA Mar 28, 2022
5c8e7db
Update test_index.py
GYHHAHA Mar 29, 2022
7330ddb
Update test_index.py
GYHHAHA Mar 29, 2022
1e1f525
Update concat.py
GYHHAHA Mar 29, 2022
6d059b4
Update v1.5.0.rst
GYHHAHA Mar 29, 2022
3c43cea
Update concat.py
GYHHAHA Mar 29, 2022
4c91709
Update test_index.py
GYHHAHA Mar 29, 2022
7429217
Update test_index.py
GYHHAHA Mar 29, 2022
3cfad33
Update test_index.py
GYHHAHA Mar 29, 2022
6ff6439
Update test_index.py
GYHHAHA Mar 29, 2022
f2fe384
add warning message check
GYHHAHA Apr 4, 2022
330187b
fix format
GYHHAHA Apr 4, 2022
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
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v1.5.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ Reshaping
- Bug in :func:`get_dummies` that selected object and categorical dtypes but not string (:issue:`44965`)
- Bug in :meth:`DataFrame.align` when aligning a :class:`MultiIndex` to a :class:`Series` with another :class:`MultiIndex` (:issue:`46001`)
- Bug in concanenation with ``IntegerDtype``, or ``FloatingDtype`` arrays where the resulting dtype did not mirror the behavior of the non-nullable dtypes (:issue:`46379`)
-
- Bug in concanenation with identical key leads to error when indexing :class:`MultiIndex` (:issue:`46519`)

Sparse
^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/reshape/concat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ def _make_concat_multiindex(indexes, keys, levels=None, names=None) -> MultiInde
names = [None]

if levels is None:
levels = [ensure_index(keys)]
levels = [ensure_index(keys).unique()]
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm shouldn't this be the case for a specified levels as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we check whether the level is unique before? If not, raise ValueError. The doc says it should be unique.

Copy link
Contributor Author

@GYHHAHA GYHHAHA Mar 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I find we actually do not have the check for depulicated levels in concat function. Something like the following will not raise. Since this problem is an isolated one, thus I will make another PR to escape from confusion.

df1 = pd.DataFrame({"A": [1]}, index=["x"])
df2 = pd.DataFrame({"A": [1]}, index=["y"])
pd.concat([df1, df2], levels=[["x", "y", "y"]]) # should raise

Copy link
Member

@rhshadrach rhshadrach Mar 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@GYHHAHA - Agreed what you are pointing out is a separate issue, but here is an example that @jreback was referring to.

df1 = pd.DataFrame({"x": [1, 2], "y": [3, 4], "z": [5, 6]}).set_index(["x", "y"])
df2 = pd.DataFrame({"x": [7, 8], "y": [9, 10], "z": [11, 12]}).set_index(["x", "y"])
result = pd.concat([df1, df2, df1], keys=["x", "y", "x"], levels=[["x", "y", "x"]])
print(result.loc['x', 1, 3])

This also raises the same error that is being addressed here.

Copy link
Contributor Author

@GYHHAHA GYHHAHA Mar 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

why you loc with string '1' and '3' instead of numeric value? @rhshadrach

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, now I get the error. I will look into this.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good! I believe you just need to apply your change to the else clause highlighted here (but I could be wrong).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I believe this is caused by duplicated levels input, if levels is [["x", "y"]], then it works fine. Maybe more suitable to add this to another PR related to unique levels keyword. @rhshadrach

Copy link
Member

Choose a reason for hiding this comment

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

Ah - I see your point; the user should not ever specify a level with duplicate values and so we can raise here instead. That makes sense to separate this off into a different PR; can you see if there is an issue for this already and open one if there isn't?

Copy link
Contributor Author

@GYHHAHA GYHHAHA Mar 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It seems that such issue doesn't exist now. I'll open one then link a pr to that after refining the performance warning check for the current PR. And also since we will raise for a duplicated level, then unique() for else clause is unnecessary.

else:
levels = [ensure_index(x) for x in levels]

Expand Down
25 changes: 25 additions & 0 deletions pandas/tests/reshape/concat/test_index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
Series,
concat,
)
from pandas.errors import PerformanceWarning
import pandas._testing as tm


Expand Down Expand Up @@ -323,3 +324,27 @@ def test_concat_multiindex_(self):
{"col": ["a", "b", "c"]}, index=MultiIndex.from_product(iterables)
)
tm.assert_frame_equal(result_df, expected_df)

@pytest.mark.parametrize(
"keys",
[["x", "y", "x"]],
)
def test_concat_with_key_not_unique(
self,
keys: list,
):
# GitHub #46519
df1 = DataFrame({"name": [1]})
df2 = DataFrame({"name": [2]})
df3 = DataFrame({"name": [3]})
df_a = concat([df1, df2, df3], keys=keys)
with tm.assert_produces_warning(PerformanceWarning):
Copy link
Contributor

Choose a reason for hiding this comment

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

what is showing the performance warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PerformanceWarning: indexing past lexsort depth may impact performance. since multi-index is unsorted.

Copy link
Member

Choose a reason for hiding this comment

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

Can you specify match="indexing past lexsort depth". This not only makes the check more specific, but also anyone reading the test will get your answer to @jreback's question.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, no problem

out_a = df_a.loc[("x", 0), :]

df_b = DataFrame(
{"name": [1, 2, 3]}, index=Index([("x", 0), ("y", 0), ("x", 0)])
)
with tm.assert_produces_warning(PerformanceWarning):
out_b = df_b.loc[("x", 0)]

tm.assert_frame_equal(out_a, out_b)