Repository: pandas-dev/pandas
Pull request: BUG-26988 implement replace for categorical blocks
- [X] closes #26988 
- [X] tests added / passed
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

The replace functions used in the code path that caused the bug report appeared to densify the series. This PR adds a replace function to `CategoricalBlock` that takes advantage of the categorical data type when possible

Problem statement:
Bug: Series[category].replace casts incorrectly
```
dti = pd.date_range('2016-01-01', periods=3, tz='US/Pacific')
ser = pd.Series(dti)
cat = ser.astype('category')

>>> cat
0   2016-01-01 00:00:00-08:00
1   2016-01-02 00:00:00-08:00
2   2016-01-03 00:00:00-08:00
dtype: category
Categories (3, datetime64[ns, US/Pacific]): [2016-01-01 00:00:00-08:00, 2016-01-02 00:00:00-08:00,
                                             2016-01-03 00:00:00-08:00]

>>> cat.replace(cat[1], 'foo')
0    1451635200000000000
1    1451721600000000000
2    1451808000000000000
dtype: int64

```

Hints from issue discussion:

Commit message: add test

Patch to review:
diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst
index 109005364fca6..c40bc5244cb8f 100644
--- a/doc/source/whatsnew/v0.25.0.rst
+++ b/doc/source/whatsnew/v0.25.0.rst
@@ -585,6 +585,7 @@ Categorical
 
 - Bug in :func:`DataFrame.at` and :func:`Series.at` that would raise exception if the index was a :class:`CategoricalIndex` (:issue:`20629`)
 - Fixed bug in comparison of ordered :class:`Categorical` that contained missing values with a scalar which sometimes incorrectly resulted in ``True`` (:issue:`26504`)
+- Bug in :func:`DataFrame.replace` and :func:`Series.replace` that would give incorrect results on categorical data (:issue:`26988`)
 -
 
 Datetimelike
diff --git a/pandas/core/internals/blocks.py b/pandas/core/internals/blocks.py
index 4cc6c86417b3b..7341fcee23511 100644
--- a/pandas/core/internals/blocks.py
+++ b/pandas/core/internals/blocks.py
@@ -2978,6 +2978,30 @@ def where(self, other, cond, align=True, errors='raise',
                                                axis=axis, transpose=transpose)
         return result
 
+    def replace(self, to_replace, value, inplace=False, filter=None,
+                regex=False, convert=True):
+        result = self if inplace else self.copy()
+        if filter is None:
+            categories = result.values.categories.tolist()
+            if to_replace in categories:
+                if isna(value):
+                    result.values.remove_categories(to_replace, inplace=True)
+                else:
+                    index = categories.index(to_replace)
+                    categories[index] = value
+                    result.values.rename_categories(categories, inplace=True)
+            if convert:
+                return result.convert(by_item=True, numeric=False,
+                                      copy=not inplace)
+            else:
+                return result
+        else:
+            if not isna(value):
+                result.values.add_categories(value, inplace=True)
+            return super(CategoricalBlock, result).replace(to_replace, value,
+                                                           inplace, filter,
+                                                           regex, convert)
+
 
 # -----------------------------------------------------------------
 # Constructor Helpers
diff --git a/pandas/tests/frame/test_replace.py b/pandas/tests/frame/test_replace.py
index 2513508822fec..a632702c3f2cd 100644
--- a/pandas/tests/frame/test_replace.py
+++ b/pandas/tests/frame/test_replace.py
@@ -1149,3 +1149,20 @@ def test_replace_method(self, to_replace, method, expected):
         result = df.replace(to_replace=to_replace, value=None, method=method)
         expected = DataFrame(expected)
         assert_frame_equal(result, expected)
+
+    @pytest.mark.parametrize("replace_dict, final_data", [
+        ({'a': 1, 'b': 1}, [[3, 3], [2, 2]]),
+        ({'a': 1, 'b': 2}, [[3, 1], [2, 3]])
+    ])
+    def test_categorical_replace_with_dict(self, replace_dict, final_data):
+        # GH 26988
+        df = DataFrame([[1, 1], [2, 2]], columns=['a', 'b'], dtype='category')
+        expected = DataFrame(final_data, columns=['a', 'b'], dtype='category')
+        expected['a'].cat.set_categories([1, 2, 3], inplace=True)
+        expected['b'].cat.set_categories([1, 2, 3], inplace=True)
+        result = df.replace(replace_dict, 3)
+        assert_frame_equal(result, expected)
+        with pytest.raises(AssertionError):
+            assert_frame_equal(df, expected)
+        df.replace(replace_dict, 3, inplace=True)
+        assert_frame_equal(df, expected)
diff --git a/pandas/tests/series/test_replace.py b/pandas/tests/series/test_replace.py
index 92096b3c95670..28a6ef088f93f 100644
--- a/pandas/tests/series/test_replace.py
+++ b/pandas/tests/series/test_replace.py
@@ -292,6 +292,25 @@ def test_replace_categorical(self, categorical, numeric):
         expected = pd.Series(numeric)
         tm.assert_series_equal(expected, result, check_dtype=False)
 
+    def test_replace_categorical_single(self):
+        # GH 26988
+        dti = pd.date_range('2016-01-01', periods=3, tz='US/Pacific')
+        s = pd.Series(dti)
+        c = s.astype('category')
+
+        expected = c.copy()
+        expected.cat.add_categories('foo', inplace=True)
+        expected[2] = 'foo'
+        expected.cat.remove_unused_categories(inplace=True)
+        assert c[2] != 'foo'
+
+        result = c.replace(c[2], 'foo')
+        tm.assert_series_equal(expected, result)
+        assert c[2] != 'foo'
+
+        c.replace(c[2], 'foo', inplace=True)
+        tm.assert_series_equal(expected, c)
+
     def test_replace_with_no_overflowerror(self):
         # GH 25616
         # casts to object without Exception from OverflowError


Code review comments:
diff_hunk: @@ -1149,3 +1149,20 @@ def test_replace_method(self, to_replace, method, expected):
         result = df.replace(to_replace=to_replace, value=None, method=method)
         expected = DataFrame(expected)
         assert_frame_equal(result, expected)
+
+    @pytest.mark.parametrize("replace_dict, final_data", [
+        ({'a': 1, 'b': 1}, [[3, 3], [2, 2]]),
+        ({'a': 1, 'b': 2}, [[3, 1], [2, 3]])
+    ])
+    def test_categorical_replace_with_dict(self, replace_dict, final_data):
+        # GH 26988
+        df = DataFrame([[1, 1], [2, 2]], columns=['a', 'b'], dtype='category')
+        expected = DataFrame(final_data, columns=['a', 'b'], dtype='category')
+        expected['a'].cat.set_categories([1, 2, 3], inplace=True)
+        expected['b'].cat.set_categories([1, 2, 3], inplace=True)
+        result = df.replace(replace_dict, 3)
+        assert_frame_equal(result, expected)
+        with pytest.raises(AssertionError):
path: pandas/tests/frame/test_replace.py
text: @user1:
can you add a comment on what the error condition is, and why would it be an AssertionError?

@author:
fixed

@user1:
wait, this is *really* odd, why is an AssertionError here?

diff_hunk: @@ -1149,3 +1149,20 @@ def test_replace_method(self, to_replace, method, expected):
         result = df.replace(to_replace=to_replace, value=None, method=method)
         expected = DataFrame(expected)
         assert_frame_equal(result, expected)
+
+    @pytest.mark.parametrize("replace_dict, final_data", [
+        ({'a': 1, 'b': 1}, [[3, 3], [2, 2]]),
+        ({'a': 1, 'b': 2}, [[3, 1], [2, 3]])
+    ])
+    def test_categorical_replace_with_dict(self, replace_dict, final_data):
+        # GH 26988
+        df = DataFrame([[1, 1], [2, 2]], columns=['a', 'b'], dtype='category')
+        expected = DataFrame(final_data, columns=['a', 'b'], dtype='category')
+        expected['a'].cat.set_categories([1, 2, 3], inplace=True)
path: pandas/tests/frame/test_replace.py
text: @user1:
try not to use inplace in tests, its non-idiomatic.