Skip to content

BUG: Ensure that fill_na in Categorical only replaces null values #27932

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 6 commits into from
Aug 16, 2019
Merged

BUG: Ensure that fill_na in Categorical only replaces null values #27932

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6ed5f08
Ensure that :func: in :class: only replaces null values
Aug 15, 2019
90bfabc
Remove argument new_categories from test function, add test case wher…
Aug 15, 2019
e088eab
Reformat whatsnew entry
MarcoGorelli Aug 15, 2019
550302a
Clean up
MarcoGorelli Aug 15, 2019
9e08857
Add test case in which categories of fill values are in different ord…
MarcoGorelli Aug 15, 2019
a679508
Correct typo and reformat whatsnew entry
MarcoGorelli Aug 16, 2019
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: 2 additions & 0 deletions doc/source/whatsnew/v0.25.1.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ Other enhancements
Bug fixes
~~~~~~~~~

- Bug in :func:`fill_na` in :class:`Categorical` would replace all values, not just those that are NaN (:issue:`26215`)


Categorical
^^^^^^^^^^^
Expand Down
6 changes: 2 additions & 4 deletions pandas/core/arrays/categorical.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1824,7 +1824,6 @@ def fillna(self, value=None, method=None, limit=None):

# pad / bfill
if method is not None:

values = self.to_dense().reshape(-1, len(self))
values = interpolate_2d(values, method, 0, None, value).astype(
self.categories.dtype
Expand All @@ -1838,10 +1837,9 @@ def fillna(self, value=None, method=None, limit=None):
if isinstance(value, ABCSeries):
if not value[~value.isin(self.categories)].isna().all():
raise ValueError("fill value must be in categories")

values_codes = _get_codes_for_values(value, self.categories)
indexer = np.where(values_codes != -1)
codes[indexer] = values_codes[values_codes != -1]
indexer = np.where(codes == -1)
codes[indexer] = values_codes[codes == -1]

# If value is not a dict or Series it should be a scalar
elif is_hashable(value):
Expand Down
22 changes: 22 additions & 0 deletions pandas/tests/series/test_missing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,28 @@ def test_fillna_categorical(self, fill_value, expected_output):
exp = Series(Categorical(expected_output, categories=["a", "b"]))
tm.assert_series_equal(s.fillna(fill_value), exp)

@pytest.mark.parametrize(
"new_categories, fill_value, expected_output",
[
(
["c", "d", "e"],
Series(["a", "b", "c", "d", "e"]),
["a", "b", "b", "d", "e"],
)
],
)
def test_fillna_categorical_with_new_categories(
self, new_categories, fill_value, expected_output
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need new_categories at all? Can you not create the series with categories=['a', 'b', 'c', 'd', 'e']?

Can you add a test where the original series and the fill_value series have the same categories, but in different order?

s = pd.Series(pd.Categorical(['a', None, 'b'], categories=['b', 'c', 'a']))
s.fillna(pd.Series(pd.Categorical(['a', 'c', 'a'], categories=['c', 'a', 'b']))

):
# GH 26215
data = ["a", np.nan, "b", np.nan, np.nan]
s = Series(Categorical(data, categories=["a", "b"]))
s.cat.add_categories(new_categories, inplace=True)
exp = Series(
Categorical(expected_output, categories=["a", "b"] + new_categories)
)
tm.assert_series_equal(s.fillna(fill_value), exp)

def test_fillna_categorical_raise(self):
data = ["a", np.nan, "b", np.nan, np.nan]
s = Series(Categorical(data, categories=["a", "b"]))
Expand Down