Skip to content

Fix pd.merge to preserve ExtensionArrays dtypes #20745

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.

Sign up for GitHub

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 5 commits into from
Apr 22, 2018
+43 −4
Merged

Fix pd.merge to preserve ExtensionArrays dtypes #20745

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
73d64eb
Fix pd.merge to preserve ExtensionArrays dtypes
jorisvandenbossche Apr 19, 2018
716e928
Fixed order.
TomAugspurger Apr 20, 2018
8824a47
Added issue number
TomAugspurger Apr 20, 2018
884510c
copy: check for arrays
jorisvandenbossche Apr 20, 2018
9cf8cfe
change order in tests for python < 3.6
jorisvandenbossche Apr 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pandas/core/dtypes/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1807,7 +1807,7 @@ def _get_dtype(arr_or_dtype):
return arr_or_dtype
elif isinstance(arr_or_dtype, type):
return np.dtype(arr_or_dtype)
elif isinstance(arr_or_dtype, CategoricalDtype):
elif isinstance(arr_or_dtype, ExtensionDtype):
return arr_or_dtype
elif isinstance(arr_or_dtype, DatetimeTZDtype):
return arr_or_dtype
Expand Down
4 changes: 2 additions & 2 deletions pandas/core/internals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5541,7 +5541,7 @@ def concatenate_join_units(join_units, concat_axis, copy):
if len(to_concat) == 1:
# Only one block, nothing to concatenate.
concat_values = to_concat[0]
if copy and concat_values.base is not None:
if copy and getattr(concat_values, 'base', 1) is not None:
Copy link
Member Author

@jorisvandenbossche jorisvandenbossche Apr 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This is kind of a hack, would like to have a better solution.

(related to discussion earlier this day in #20721 (comment) about deprecating Index.base)

Copy link
Contributor

@jreback jreback Apr 19, 2018

Choose a reason for hiding this comment

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

you should only check base if concat_values is an ndarray

Copy link
Member Author

@jorisvandenbossche jorisvandenbossche Apr 20, 2018

Choose a reason for hiding this comment

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

@jreback updated

concat_values = concat_values.copy()
else:
concat_values = _concat._concat_compat(to_concat, axis=concat_axis)
Expand Down Expand Up @@ -5823,7 +5823,7 @@ def get_reindexed_values(self, empty_dtype, upcasted_na):
# External code requested filling/upcasting, bool values must
# be upcasted to object to avoid being upcasted to numeric.
values = self.block.astype(np.object_).values
elif self.block.is_categorical:
elif self.block.is_extension:
values = self.block.values
else:
# No dtype upcasting is done here, it will be performed during
Expand Down
21 changes: 21 additions & 0 deletions pandas/tests/extension/base/reshaping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,24 @@ def test_set_frame_overwrite_object(self, data):
df = pd.DataFrame({"A": [1] * len(data)}, dtype=object)
df['A'] = data
assert df.dtypes['A'] == data.dtype

def test_merge(self, data, na_value):
Copy link
Contributor

@jreback jreback Apr 21, 2018

Choose a reason for hiding this comment

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

you prob should test with with the how=join_type fixture.

Copy link
Member Author

@jorisvandenbossche jorisvandenbossche Apr 21, 2018

Choose a reason for hiding this comment

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

They both need a different expected result, I don't think it is really worth it here in this case?
(the test is also not meant to be a full cover of the merge function (for that we already have other tests), just to test that basic use cases of concatting works with extension arrays)

Copy link
Contributor

@jreback jreback Apr 22, 2018

Choose a reason for hiding this comment

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

ok, its worth doing way more tests than a single usecase. but ok here I guess.


Copy link
Contributor

@jreback jreback Apr 19, 2018

Choose a reason for hiding this comment

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

issue number

df1 = pd.DataFrame({'int1': [1, 2, 3], 'key': [0, 1, 2],
'ext': data[:3]})
df2 = pd.DataFrame({'int2': [1, 2, 3, 4], 'key': [0, 0, 1, 3]})

res = pd.merge(df1, df2)
exp = pd.DataFrame(
{'int1': [1, 1, 2], 'int2': [1, 2, 3], 'key': [0, 0, 1],
'ext': data._constructor_from_sequence(
[data[0], data[0], data[1]])})
self.assert_frame_equal(res, exp[['ext', 'int1', 'key', 'int2']])

res = pd.merge(df1, df2, how='outer')
exp = pd.DataFrame(
{'int1': [1, 1, 2, 3, np.nan], 'int2': [1, 2, 3, np.nan, 4],
'key': [0, 0, 1, 2, 3],
'ext': data._constructor_from_sequence(
[data[0], data[0], data[1], data[2], na_value])})
self.assert_frame_equal(res, exp[['ext', 'int1', 'key', 'int2']])
4 changes: 4 additions & 0 deletions pandas/tests/extension/category/test_categorical.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ def test_align(self, data, na_value):
def test_align_frame(self, data, na_value):
pass

@pytest.mark.skip(reason="Unobserved categories preseved in concat.")
def test_merge(self, data, na_value):
pass


class TestGetitem(base.BaseGetitemTests):
@pytest.mark.skip(reason="Backwards compatibility")
Expand Down