-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
ENH: add error message for merge with Series GH12081 #12112
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
@@ -261,6 +261,20 @@ def test_join_on_fails_with_different_column_counts(self): | |||
index=tm.makeCustomIndex(10, 2)) | |||
merge(df, df2, right_on='a', left_on=['a', 'b']) | |||
|
|||
def test_join_on_fails_with_left_series(self): | |||
with tm.assertRaisesRegexp(ValueError, 'Series'): | |||
series = Series([0, 1, 2]) |
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.
iterate thru a bunch of things (e.g. ndarray, number, string) and assert that they raise as well
e609393
to
dcff8ca
Compare
@jreback incorporated your comments |
@@ -261,6 +261,52 @@ def test_join_on_fails_with_different_column_counts(self): | |||
index=tm.makeCustomIndex(10, 2)) | |||
merge(df, df2, right_on='a', left_on=['a', 'b']) | |||
|
|||
|
|||
def test_join_on_fails_with_left_series(self): | |||
# GH12081 |
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.
you can put all of these test cases in a single test with a loop
5f06cd1
to
74bb643
Compare
@jreback true, looks much nicer now. |
looks good. can you add a whatsnew note in API changes. |
74bb643
to
40ced26
Compare
@jreback I added the note to whatsnew. I didn't manage to run Sphinx to test build the doc though. Can you give it a spin? I'll have to clean my development environment and move from using pip to conda. Just don't really have the time now. |
@dan-tee no looks good. thanks for the PR! |
closes #12081