Skip to content
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

BUG: nullable dtypes not preserved in Series.replace #44940

Merged
merged 9 commits into from
Dec 19, 2021

Conversation

jbrockmendel
Copy link
Member

@jbrockmendel jbrockmendel commented Dec 17, 2021

Sits on top of #44932

test_replace_dtype is cribbed from #37512

Improves inconsistent Categorical behavior while removing a special-case kludge for Categorical in replace_list.

Fixes an inconsistent int->float behavior introduced (intentionally, the inconsistency wasn't noticed) in #15742

@jreback jreback added Bug Dtype Conversions Unexpected or buggy dtype conversions Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Dec 17, 2021
@jreback jreback added this to the 1.4 milestone Dec 17, 2021
@jreback
Copy link
Contributor

jreback commented Dec 17, 2021

lgtm. let's get a few more eyes on this.

cc @pandas-dev/pandas-core if any comments

@jbrockmendel
Copy link
Member Author

updated with tests for some more closed issues

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.

question on categoricals

doc/source/whatsnew/v1.4.0.rst Show resolved Hide resolved
# GH 24971, GH#23305
ser = pd.Series(categorical)
result = ser.replace({"A": 1, "B": 2})
expected = pd.Series(numeric).astype("category")
Copy link
Contributor

Choose a reason for hiding this comment

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

umm why is this a resulting categorical?

Copy link
Member Author

Choose a reason for hiding this comment

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

bc that's what someone implemented for the Categorical.replace behavior. im on the fence about it, but for now i think we need to be consistent with it

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm ok let's open an issue about this, i dont' think we should raise (or coerce to object) rather than return a new categorical (but maybe others disagree)

Copy link
Member Author

Choose a reason for hiding this comment

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

can you clarify? based on your earlier comment i expected the opposite opinion, so i think the "dont" may be an unintentional double-negative.

@jreback jreback merged commit 9cd1c6f into pandas-dev:master Dec 19, 2021
@jreback
Copy link
Contributor

jreback commented Dec 19, 2021

thanks followup comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment