Skip to content
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

[TST]: Add test for duplicate keys in concat #36556

Merged
merged 5 commits into from Nov 26, 2020
Merged

Conversation

phofl
Copy link
Member

@phofl phofl commented Sep 22, 2020

Added a test

@phofl phofl added the Reshaping Concat, Merge/Join, Stack/Unstack, Explode label Sep 22, 2020
@phofl
Copy link
Member Author

phofl commented Sep 22, 2020

Test failure seems unrelated

@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Oct 23, 2020
# GH: 20816
series_list = [pd.Series({"a": 1}), pd.Series({"b": 2}), pd.Series({"c": 3})]
result = pd.concat(series_list, keys=keys, verify_integrity=integrity)
tuples = [(first, second) for first, second in zip(keys, ["a", "b", "c"])]
Copy link
Member

Choose a reason for hiding this comment

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

I guess this could be simplified.

tuples = list(zip(keys, ["a", "b", "c"]))

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, thank you very much

Copy link
Member

@ivanovmg ivanovmg left a comment

Choose a reason for hiding this comment

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

Couple of minor comments, overall LGTM.

Comment on lines 2932 to 2939
@pytest.mark.parametrize(
("keys", "integrity"),
[
(["red"] * 3, False),
(["red", "blue", "red"], False),
(["red", "blue", "red"], True),
],
)
Copy link
Member

Choose a reason for hiding this comment

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

Was there a plan to follow the same exact parametrization as in the referenced github issue?
I would suggest to have these tests with verify_integrity=True/False for all keys.

Copy link
Member Author

Choose a reason for hiding this comment

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

Was not mentioned as a problem, so I think that was the reason I did not add it. But was more than 4 weeks ago, so I am not quite sure. Added it now.

@jreback
Copy link
Contributor

jreback commented Nov 26, 2020

can you merge master and ping on green.

@jreback jreback added Testing pandas testing functions or related to the test suite and removed Stale labels Nov 26, 2020
@phofl
Copy link
Member Author

phofl commented Nov 26, 2020

@jreback green

@jreback jreback added this to the 1.2 milestone Nov 26, 2020
@jreback jreback merged commit 11a5a4b into pandas-dev:master Nov 26, 2020
@jreback
Copy link
Contributor

jreback commented Nov 26, 2020

thanks!

@phofl phofl deleted the 20816 branch November 27, 2020 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Reshaping Concat, Merge/Join, Stack/Unstack, Explode Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pd.concat fails if given repeated keys
3 participants