API: Validate keyword arguments to fillna

Closes pandas-dev#19682

Author: TomAugspurger

doc/source/whatsnew/v0.23.0.txt

@@ -570,6 +570,7 @@ Other API Changes
 - Set operations (union, difference...) on :class:`IntervalIndex` with incompatible index types will now raise a ``TypeError`` rather than a ``ValueError`` (:issue:`19329`)
 - :class:`DateOffset` objects render more simply, e.g. "<DateOffset: days=1>" instead of "<DateOffset: kwds={'days': 1}>" (:issue:`19403`)
 - :func:`pandas.merge` provides a more informative error message when trying to merge on timezone-aware and timezone-naive columns (:issue:`15800`)
+- ``Categorical.fillna`` now validates its ``value`` and ``method`` keyword arguments. It now raises when both or none are specified, matching the behavior of :meth:`Series.fillna` (:issue:`19682`)
 
 .. _whatsnew_0230.deprecations:
 

pandas/core/arrays/categorical.py

@@ -40,7 +40,7 @@
     Appender, cache_readonly, deprecate_kwarg, Substitution)
 
 from pandas.io.formats.terminal import get_terminal_size
-from pandas.util._validators import validate_bool_kwarg
+from pandas.util._validators import validate_bool_kwarg, validate_fillna_kwargs
 from pandas.core.config import get_option
 
 from .base import ExtensionArray
@@ -1607,6 +1607,9 @@ def fillna(self, value=None, method=None, limit=None):
         -------
         filled : Categorical with NA/NaN filled
         """
+        value, method = validate_fillna_kwargs(
+            value, method, validate_scalar_dict_value=False
+        )
 
         if value is None:
             value = np.nan

pandas/core/generic.py

@@ -54,7 +54,7 @@
 import pandas.core.nanops as nanops
 from pandas.util._decorators import (Appender, Substitution,
                                      deprecate_kwarg)
-from pandas.util._validators import validate_bool_kwarg
+from pandas.util._validators import validate_bool_kwarg, validate_fillna_kwargs
 from pandas.core import config
 
 # goal is to be able to define the docs close to function, while still being
@@ -4694,10 +4694,8 @@ def infer_objects(self):
     def fillna(self, value=None, method=None, axis=None, inplace=False,
                limit=None, downcast=None):
         inplace = validate_bool_kwarg(inplace, 'inplace')
+        value, method = validate_fillna_kwargs(value, method)
 
-        if isinstance(value, (list, tuple)):
-            raise TypeError('"value" parameter must be a scalar or dict, but '
-                            'you passed a "{0}"'.format(type(value).__name__))
         self._consolidate_inplace()
 
         # set the default here, so functions examining the signaure
@@ -4708,8 +4706,7 @@ def fillna(self, value=None, method=None, axis=None, inplace=False,
         method = missing.clean_fill_method(method)
         from pandas import DataFrame
         if value is None:
-            if method is None:
-                raise ValueError('must specify a fill method or value')
+
             if self._is_mixed_type and axis == 1:
                 if inplace:
                     raise NotImplementedError()
@@ -4743,9 +4740,6 @@ def fillna(self, value=None, method=None, axis=None, inplace=False,
                                                   coerce=True,
                                                   downcast=downcast)
         else:
-            if method is not None:
-                raise ValueError('cannot specify both a fill method and value')
-
             if len(self._get_axis(axis)) == 0:
                 return self
 

pandas/tests/categorical/test_missing.py

@@ -53,3 +53,23 @@ def test_set_item_nan(self):
 
         exp = Categorical([1, np.nan, 3], categories=[1, 2, 3])
         tm.assert_categorical_equal(cat, exp)
+
+    def test_fillna_raises(self):
+        # https://github.com/pandas-dev/pandas/issues/19682
+
+        cat = Categorical([1, 2, 3])
+
+        xpr = "Cannot specify both 'value' and 'method'."
+
+        with tm.assert_raises_regex(ValueError, xpr):
+            cat.fillna(value=1, method='ffill')
+
+        xpr = "Must specify a fill 'value' or 'method'."
+
+        with tm.assert_raises_regex(ValueError, xpr):
+            cat.fillna()
+
+        xpr = "Invalid fill method. Expecting .* bad"
+
+        with tm.assert_raises_regex(ValueError, xpr):
+            cat.fillna(method='bad')

pandas/util/_validators.py

@@ -320,3 +320,37 @@ def validate_axis_style_args(data, args, kwargs, arg_name, method_name):
         msg = "Cannot specify all of '{}', 'index', 'columns'."
         raise TypeError(msg.format(arg_name))
     return out
+
+
+def validate_fillna_kwargs(value, method, validate_scalar_dict_value=True):
+    """Validate the keyword arguments to 'fillna'.
+
+    This checks that exactly one of 'value' and 'method' is specified.
+    If 'method' is specified, this validates that it's a valid method.
+
+    Parameters
+    ----------
+    value, method : object
+        The 'value' and 'method' keyword arguments for 'fillna'.
+
+    Returns
+    -------
+    value, method : object
+        These aren't modified in any way. Just validated.
+    """
+    from pandas.core.missing import clean_fill_method
+
+    if value is None and method is None:
+        raise ValueError("Must specify a fill 'value' or 'method'.")
+    elif value is None and method is not None:
+        clean_fill_method(method)
+
+    elif value is not None and method is None:
+        if validate_scalar_dict_value and isinstance(value, (list, tuple)):
+            raise TypeError('"value" parameter must be a scalar or dict, but '
+                            'you passed a "{0}"'.format(type(value).__name__))
+
+    elif value is not None and method is not None:
+        raise ValueError("Cannot specify both 'value' and 'method'.")
+
+    return value, method