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

DEPR: categorical.take allow_fill default #29912

Merged
merged 5 commits into from
Nov 29, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
3 changes: 2 additions & 1 deletion doc/source/whatsnew/v1.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ Deprecations
is equivalent to ``arr[idx.get_loc(idx_val)] = val``, which should be used instead (:issue:`28621`).
- :func:`is_extension_type` is deprecated, :func:`is_extension_array_dtype` should be used instead (:issue:`29457`)
- :func:`eval` keyword argument "truediv" is deprecated and will be removed in a future version (:issue:`29812`)
- :meth:`Categorical.take_nd` is deprecated, use :meth:`Categorical.take` instead (:issue:`27745`)
Copy link
Contributor

Choose a reason for hiding this comment

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

this was never public to begin with, but I guess can't hurt.


.. _whatsnew_1000.prior_deprecations:

Expand Down Expand Up @@ -455,10 +456,10 @@ or ``matplotlib.Axes.plot``. See :ref:`plotting.formatters` for more.
- In :func:`concat` the default value for ``sort`` has been changed from ``None`` to ``False`` (:issue:`20613`)
- Removed previously deprecated "raise_conflict" argument from :meth:`DataFrame.update`, use "errors" instead (:issue:`23585`)
- Removed previously deprecated keyword "n" from :meth:`DatetimeIndex.shift`, :meth:`TimedeltaIndex.shift`, :meth:`PeriodIndex.shift`, use "periods" instead (:issue:`22458`)
- Changed the default ``fill_value`` in :meth:`Categorical.take` from ``True`` to ``False`` (:issue:`20841`)
- Changed the default value for the `raw` argument in :func:`Series.rolling().apply() <pandas.core.window.Rolling.apply>`, :func:`DataFrame.rolling().apply() <pandas.core.window.Rolling.apply>`,
- :func:`Series.expanding().apply() <pandas.core.window.Expanding.apply>`, and :func:`DataFrame.expanding().apply() <pandas.core.window.Expanding.apply>` to ``False`` (:issue:`20584`)
- Removed previously deprecated :attr:`Timestamp.weekday_name`, :attr:`DatetimeIndex.weekday_name`, and :attr:`Series.dt.weekday_name` (:issue:`18164`)
-

.. _whatsnew_1000.performance:

Expand Down
36 changes: 12 additions & 24 deletions pandas/core/arrays/categorical.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import operator
from shutil import get_terminal_size
import textwrap
from typing import Type, Union, cast
from warnings import warn

Expand Down Expand Up @@ -59,18 +58,6 @@

from .base import ExtensionArray, _extension_array_shared_docs, try_cast_to_ea

_take_msg = textwrap.dedent(
"""\
Interpreting negative values in 'indexer' as missing values.
In the future, this will change to meaning positional indices
from the right.

Use 'allow_fill=True' to retain the previous behavior and silence this
warning.

Use 'allow_fill=False' to accept the new behavior."""
)


def _cat_compare_op(op):
opname = f"__{op.__name__}__"
Expand Down Expand Up @@ -1829,7 +1816,7 @@ def fillna(self, value=None, method=None, limit=None):

return self._constructor(codes, dtype=self.dtype, fastpath=True)

def take_nd(self, indexer, allow_fill=None, fill_value=None):
def take(self, indexer, allow_fill: bool = False, fill_value=None):
"""
Take elements from the Categorical.

Expand All @@ -1838,7 +1825,7 @@ def take_nd(self, indexer, allow_fill=None, fill_value=None):
indexer : sequence of int
The indices in `self` to take. The meaning of negative values in
`indexer` depends on the value of `allow_fill`.
allow_fill : bool, default None
allow_fill : bool, default False
How to handle negative values in `indexer`.

* False: negative values in `indices` indicate positional indices
Expand All @@ -1849,11 +1836,9 @@ def take_nd(self, indexer, allow_fill=None, fill_value=None):
(the default). These values are set to `fill_value`. Any other
other negative values raise a ``ValueError``.

.. versionchanged:: 0.23.0
.. versionchanged:: 1.0.0

Deprecated the default value of `allow_fill`. The deprecated
default is ``True``. In the future, this will change to
``False``.
Default value changed from ``True`` to ``False``.

fill_value : object
The value to use for `indices` that are missing (-1), when
Expand Down Expand Up @@ -1903,10 +1888,6 @@ def take_nd(self, indexer, allow_fill=None, fill_value=None):
will raise a ``TypeError``.
"""
indexer = np.asarray(indexer, dtype=np.intp)
if allow_fill is None:
if (indexer < 0).any():
warn(_take_msg, FutureWarning, stacklevel=2)
allow_fill = True

dtype = self.dtype

Expand All @@ -1927,7 +1908,14 @@ def take_nd(self, indexer, allow_fill=None, fill_value=None):
result = type(self).from_codes(codes, dtype=dtype)
return result

take = take_nd
def take_nd(self, indexer, allow_fill: bool = False, fill_value=None):
# GH#27745 deprecate alias that other EAs dont have
warn(
"Categorical.take_nd is deprecated, use Categorical.take instead",
FutureWarning,
stacklevel=2,
)
return self.take(indexer, allow_fill=allow_fill, fill_value=fill_value)

def __len__(self) -> int:
"""
Expand Down
13 changes: 10 additions & 3 deletions pandas/tests/arrays/categorical/test_algos.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,12 @@ def test_isin_empty(empty):
class TestTake:
# https://github.com/pandas-dev/pandas/issues/20664

def test_take_warns(self):
def test_take_default_allow_fill(self):
cat = pd.Categorical(["a", "b"])
with tm.assert_produces_warning(FutureWarning):
cat.take([0, -1])
with tm.assert_produces_warning(None):
result = cat.take([0, -1])

assert result.equals(cat)

def test_take_positive_no_warning(self):
cat = pd.Categorical(["a", "b"])
Expand Down Expand Up @@ -158,3 +160,8 @@ def test_take_fill_value_new_raises(self):
xpr = r"'fill_value' \('d'\) is not in this Categorical's categories."
with pytest.raises(TypeError, match=xpr):
cat.take([0, 1, -1], fill_value="d", allow_fill=True)

def test_take_nd_deprecated(self):
cat = pd.Categorical(["a", "b", "c"])
with tm.assert_produces_warning(FutureWarning):
cat.take_nd([0, 1])