Skip to content

Added two tests for issue #29697 #33508

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

Merged
merged 4 commits into from
Apr 26, 2020
Merged

Conversation

devjeetr
Copy link
Contributor

@devjeetr devjeetr commented Apr 13, 2020

I added two basic tests for merge as requested in #29697, one for how="outer" and one for how="right".

I'm not sure if the way I'm creating duplicate columns for the expected dataframe is the recommended way but I could not find anything on this. Please let me know if there's a better way to do this.

@alimcmaster1 alimcmaster1 added the Testing pandas testing functions or related to the test suite label Apr 14, 2020
@@ -645,6 +645,39 @@ def _constructor(self):

assert isinstance(result, NotADataFrame)

def test_merge_right_duplicate_suffix(self):
Copy link
Member

Choose a reason for hiding this comment

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

These tests are very similar could you use @pytest.mark.parameterize here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added @pytest.mark.parameterize. I am not super familiar with pytest but I tried following other tests that used parameterize, but let me know about any changes I should make.

@alimcmaster1
Copy link
Member

Think the best place in the file for this test method would be directly after def test_merge_suffix

@devjeetr
Copy link
Contributor Author

Think the best place in the file for this test method would be directly after def test_merge_suffix

I gave it a look and I agree. Moved it in the latest commit.

@devjeetr devjeetr requested a review from alimcmaster1 April 16, 2020 06:04
@pep8speaks
Copy link

pep8speaks commented Apr 16, 2020

Hello @devjeetr! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-04-23 18:29:46 UTC

@alimcmaster1
Copy link
Member

LGTM please fix up the pep8 issues for CI to pass. @devjeetr

@jbrockmendel
Copy link
Member

force-pushed a lint fixup

@jreback jreback added the Reshaping Concat, Merge/Join, Stack/Unstack, Explode label Apr 26, 2020
@jreback jreback added this to the 1.1 milestone Apr 26, 2020
@jreback jreback merged commit f3fdab3 into pandas-dev:master Apr 26, 2020
@jreback
Copy link
Contributor

jreback commented Apr 26, 2020

thanks @devjeetr

rhshadrach pushed a commit to rhshadrach/pandas that referenced this pull request May 10, 2020
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.

BUG: merge raises for how='outer'/'right' when duplicate suffixes are specified
5 participants