Skip to content

REGR: Fix bug when replacing categorical value with self #33292

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

Merged
merged 6 commits into from
Apr 6, 2020
Merged

REGR: Fix bug when replacing categorical value with self #33292

merged 6 commits into from
Apr 6, 2020

Conversation

dsaxton
Copy link
Member

@dsaxton dsaxton commented Apr 4, 2020

@jbrockmendel
Copy link
Member

LGTM pending green

@ShaharNaveh
Copy link
Member

@dsaxton If you rebase it should fix the Travis issue

@simonjayhawkins simonjayhawkins added this to the 1.0.4 milestone Apr 5, 2020
@simonjayhawkins
Copy link
Member

@dsaxton I've tagged this as 1.0.4 for now since #33288 is a regression.

I don't think need to move whatsnew yet, see #33157 (comment)

opened #33300 to help keep track and discuss

@simonjayhawkins simonjayhawkins added Regression Functionality that used to work in a prior pandas version Categorical Categorical Data Type labels Apr 5, 2020
@simonjayhawkins simonjayhawkins changed the title BUG: Fix bug when replacing categorical value with self REGR: Fix bug when replacing categorical value with self Apr 5, 2020
@jreback jreback modified the milestones: 1.0.4, 1.1 Apr 5, 2020
@@ -2463,7 +2463,8 @@ def replace(self, to_replace, value, inplace: bool = False):
if new_value in cat.categories:
Copy link
Contributor

Choose a reason for hiding this comment

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

you can move this check to line 2451, e.g.

elif replace_value == new_value:
    continue

@@ -64,6 +64,7 @@ def test_isin_cats():
[
("b", "c", ["a", "c"], "Categorical.categories are different"),
("c", "d", ["a", "b"], None),
("a", "a", ["a", "b"], None),
Copy link
Contributor

Choose a reason for hiding this comment

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

can you annotate this case (add a comment on line before with this issue number)

@jbrockmendel
Copy link
Member

@dsaxton if other comments are addressed, can you re-push? the docbuild should be OK now

@jreback jreback merged commit bffafbb into pandas-dev:master Apr 6, 2020
@jreback
Copy link
Contributor

jreback commented Apr 6, 2020

thanks @dsaxton

@dsaxton dsaxton deleted the replace-cat branch April 6, 2020 22:18
jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request Apr 11, 2020
simonjayhawkins pushed a commit to simonjayhawkins/pandas that referenced this pull request May 5, 2020
simonjayhawkins added a commit that referenced this pull request May 5, 2020
…gorical value with self) (#34004)

Co-authored-by: Daniel Saxton <2658661+dsaxton@users.noreply.github.com>
@simonjayhawkins simonjayhawkins modified the milestones: 1.1, 1.0.4 May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Categorical Categorical Data Type Regression Functionality that used to work in a prior pandas version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

REGR: Replacing a category with itself replaces it with np.nan
5 participants