-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
Conversation
@@ -645,6 +645,39 @@ def _constructor(self): | |||
|
|||
assert isinstance(result, NotADataFrame) | |||
|
|||
def test_merge_right_duplicate_suffix(self): |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Think the best place in the file for this test method would be directly after |
I gave it a look and I agree. Moved it in the latest commit. |
LGTM please fix up the pep8 issues for CI to pass. @devjeetr |
force-pushed a lint fixup |
thanks @devjeetr |
I added two basic tests for
merge
as requested in #29697, one forhow="outer"
and one forhow="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.black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff