-
-
Notifications
You must be signed in to change notification settings - Fork 19.1k
ENH: allow attrs to be propagated via pd.concat #42252
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
56e1d4f
93da2b2
2043634
23541a8
b349e96
d2ba70f
60cce3b
757726b
28614cb
bb59b1b
298e572
e172863
e41822f
ab34f2f
49bd5b1
2c2e0d1
cfcc6d1
cadfb63
dd19672
93cc257
e679918
08e6fab
f49f027
0aee5e5
cefc444
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 | ||
---|---|---|---|---|
|
@@ -347,7 +347,6 @@ | |||
(pd.Series, ([1, 2],), operator.methodcaller("convert_dtypes")), | ||||
pytest.param( | ||||
(pd.DataFrame, frame_data, operator.methodcaller("convert_dtypes")), | ||||
marks=not_implemented_mark, | ||||
), | ||||
(pd.Series, ([1, None, 3],), operator.methodcaller("interpolate")), | ||||
(pd.DataFrame, ({"A": [1, None, 3]},), operator.methodcaller("interpolate")), | ||||
|
@@ -762,7 +761,7 @@ def test_groupby_finalize(obj, method): | |||
[ | ||||
lambda x: x.agg(["sum", "count"]), | ||||
lambda x: x.transform(lambda y: y), | ||||
lambda x: x.apply(lambda y: y), | ||||
# lambda x: x.apply(lambda y: y), Fixed with #42252 | ||||
|
concatenated = concat(results, keys=keys, axis=1, sort=False) |
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.
Yet, I am not sure I understood the reason.
Maybe some other reviewers will catch the idea.
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.
@ivanovmg Sorry for the confusion. The test checks if apply
will retain the attrs
and mark this as failing as the attrs
should get lost.
However, in this test, the apply
calls concat
, which will now preserve the attrs
.
Though the function apply
don't have a finaliser to ensure the pass of attrs
, all the operations done inside apply
will preserve the attrs
. Thus, the final result will have attrs
and thus, the test will no longer fail. So I have removed the mark as fail.
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.
don't comment out cases
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -653,3 +653,30 @@ def test_concat_posargs_deprecation(): | |
result = concat([df, df2], 0) | ||
expected = DataFrame([[1, 2, 3], [4, 5, 6]], index=["a", "b"]) | ||
tm.assert_frame_equal(result, expected) | ||
|
||
|
||
def test_concat_retain_attrs(): | ||
''' Retain the attrs during concat | ||
|
||
Only retain the attrs when the attrs are the same across all dataframes.''' | ||
|
||
d = {'col1': [1, 2], 'col2': [3, 4]} | ||
df1 = pd.DataFrame(data=d) | ||
df1.attrs = {1: 1} | ||
df2 = pd.DataFrame(data=d) | ||
df2.attrs = {1: 1} | ||
df = pd.concat([df1, df2]) | ||
assert df.attrs == {1: 1} | ||
|
||
|
||
|
||
def test_concat_drop_attrs(): | ||
|
||
'''Discard attrs when they don't match. | ||
|
||
Drop the attrs when the attrs when the attrs are different across | ||
all dataframes.''' | ||
d = {'col1': [1, 2], 'col2': [3, 4]} | ||
df1 = pd.DataFrame(data=d) | ||
df1.attrs = {1: 1} | ||
df2 = pd.DataFrame(data=d) | ||
df2.attrs = {1: 2} | ||
df = pd.concat([df1, df2]) | ||
assert df.attrs == {} |
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.
this is already handled above no? L5448
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.
@jreback Thanks for the review. No.
So during concatenation, the
other
is aconcatenator
, which is not anNDFrame
.So it will not pass the if statement in L5447.
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.
can remove the comment