Skip to content

BUG: Fixed concat with reindex and extension types #33522

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

Jump to bottom
Merged
merged 9 commits into from
Jul 1, 2020
Merged

BUG: Fixed concat with reindex and extension types #33522

merged 9 commits into from
Jul 1, 2020

Conversation

TomAugspurger
Copy link
Contributor

Closes #27692
Closes #33027

I have a larger cleanup planned, but this fixes the linked issues for now.

@TomAugspurger TomAugspurger added this to the 1.1 milestone Apr 13, 2020
@TomAugspurger TomAugspurger added Reshaping Concat, Merge/Join, Stack/Unstack, Explode ExtensionArray Extending pandas with custom dtypes or arrays. labels Apr 13, 2020
@jbrockmendel
Copy link
Member

I have a larger cleanup planned

Can you give a brief overview? ive got a couple of branches going, may need to mothball them until you're done

@TomAugspurger
Copy link
Contributor Author

Can you give a brief overview?

Nothing actually written down yet, and no timeline. Just a general sense of "this is probably more complex than it needs to be". So I wouldn't recommend planning around me.

@jbrockmendel
Copy link
Member

Might conflict/overlap with #33486

jreback
jreback requested changes Apr 16, 2020
View reviewed changes
@jbrockmendel
Copy link
Member

needs rebase

@jbrockmendel
Copy link
Member

just pushed a rebase

@jorisvandenbossche jorisvandenbossche mentioned this pull request May 23, 2020
Merged
@TomAugspurger
Copy link
Contributor Author

Just (force) pushed a rebase.

I think this will fail till #34338 is in.

@jorisvandenbossche
Copy link
Member

@TomAugspurger this will need an update now #34339 is merged

@TomAugspurger
Copy link
Contributor Author

Merged. No need for ignore_2d_ea now that #34339 is in.

jreback
jreback requested changes Jun 20, 2020
View reviewed changes
@TomAugspurger
Copy link
Contributor Author

This should be good to go now.

jreback
jreback requested changes Jun 30, 2020
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

tiny comment, otherwise lgtm.

pandas/core/internals/concat.py
@@ -374,6 +384,10 @@ def _get_empty_dtype_and_na(join_units):
upcast_cls = "category"
elif is_datetime64tz_dtype(dtype):
upcast_cls = "datetimetz"

Copy link
Contributor

Choose a reason for hiding this comment

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

sort of dumb, but no blank lines before/after

@jreback jreback merged commit bf12604 into pandas-dev:master Jul 1, 2020
@jreback
Copy link
Contributor

jreback commented Jul 1, 2020

thanks @TomAugspurger

jorisvandenbossche
jorisvandenbossche reviewed Jul 2, 2020
View reviewed changes
pandas/tests/extension/test_categorical.py
@@ -93,7 +93,8 @@ class TestConstructors(base.BaseConstructorsTests):


class TestReshaping(base.BaseReshapingTests):
pass
def test_concat_with_reindex(self, data):
pytest.xfail(reason="Deliberately upcast to object?")
Copy link
Member

Choose a reason for hiding this comment

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

Do we have an issue for this? (it seems we shouldn't cast to object when only needing to append NaNs?)

@jorisvandenbossche jorisvandenbossche mentioned this pull request Jul 15, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorisvandenbossche jorisvandenbossche jorisvandenbossche left review comments

@jbrockmendel jbrockmendel jbrockmendel left review comments

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
ExtensionArray Extending pandas with custom dtypes or arrays. Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Milestone
1.1
4 participants
@TomAugspurger @jbrockmendel @jorisvandenbossche @jreback