-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ENH: Merge DataFrame and Series using on
(GH21220)
#21223
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
Changes from 6 commits
91a3f16
64e6e66
8939fad
b09ffb7
1a0e5a2
59b487d
0d14cc2
12da2ce
2d689ad
687568b
7ed9688
2ce56e7
ac9d5a1
fdf324c
e896a91
69d8bda
f7b333f
c4c988c
d099fd6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -230,7 +230,10 @@ def test_join_on_fails_with_different_column_counts(self): | |
|
||
def test_join_on_fails_with_wrong_object_type(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you parameterize this test |
||
# GH12081 | ||
wrongly_typed = [Series([0, 1]), 2, 'str', None, np.array([0, 1])] | ||
""" GH21220 - merging of Series and DataFrame is now allowed | ||
Edited the test to remove the Series object from 'wrongly_typed' | ||
""" | ||
wrongly_typed = [2, 'str', None, np.array([0, 1])] | ||
df = DataFrame({'a': [1, 1]}) | ||
|
||
for obj in wrongly_typed: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1864,3 +1864,42 @@ def test_merge_index_types(index): | |
OrderedDict([('left_data', [1, 2]), ('right_data', [1.0, 2.0])]), | ||
index=index) | ||
assert_frame_equal(result, expected) | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also parameterize this as much as possible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
@pytest.mark.parametrize("on,left_on,right_on,left_index,right_index,nms,nm", [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you format this so it lines up, its pretty unreadable now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks - done |
||
(['outer', 'inner'], None, None, False, False, | ||
['outer', 'inner'], 'B'), | ||
(None, None, None, True, True, ['outer', 'inner'], | ||
'B'), | ||
(None, ['outer', 'inner'], None, False, True, None, | ||
'B'), | ||
(None, None, ['outer', 'inner'], True, False, None, | ||
'B'), | ||
(['outer', 'inner'], None, None, False, False, | ||
['outer', 'inner'], None), | ||
(None, None, None, True, True, ['outer', 'inner'], | ||
None), | ||
(None, ['outer', 'inner'], None, False, True, None, | ||
None), | ||
(None, None, ['outer', 'inner'], True, False, None, | ||
None), | ||
]) | ||
def test_merge_series(on, left_on, right_on, left_index, right_index, nms, nm): | ||
# GH 21220 | ||
a = pd.DataFrame({"A": [1, 2, 3, 4]}, | ||
index=pd.MultiIndex.from_product([['a', 'b'], [0, 1]], | ||
names=['outer', 'inner'])) | ||
b = pd.Series([1, 2, 3, 4], | ||
index=pd.MultiIndex.from_product([['a', 'b'], [1, 2]], | ||
names=['outer', 'inner']), name=nm) | ||
expected = pd.DataFrame({"A": [2, 4], "B": [1, 3]}, | ||
index=pd.MultiIndex.from_product([['a', 'b'], [1]], | ||
names=nms)) | ||
if nm is not None: | ||
result = pd.merge(a, b, on=on, left_on=left_on, right_on=right_on, | ||
left_index=left_index, right_index=right_index) | ||
tm.assert_frame_equal(result, expected) | ||
else: | ||
with tm.assert_raises_regex(ValueError, 'a Series without a name'): | ||
result = pd.merge(a, b, on=on, left_on=left_on, right_on=right_on, | ||
left_index=left_index, right_index=right_index) |
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.
just raise, and more like
Can only merge Series or DataFrame objects, a {obj} was passed
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.
Thanks - done - updated the statement and made into a TypeError
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.
Is there a reason you changed this to TypeError?
I would personally keep it as ValueError
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.
@jorisvandenbossche Thanks - I thought TypeError is more relevant for the statement below, as the message is telling the user "only objects of type Series or DataFrame can be merged"
Should I revert it back to ValueError?
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.
I don't think the one is necessarily much better than the other in this case, so I would personally rather keep it as it was to be conservative.
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.
Although eg
concat
raises TypeError if the input is not series or dataframe, so maybe TypeError is more consistent with pandas.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.
Ok - not changing then and keeping it (i.e. TypeError)