Skip to content

Commit

Permalink
DEPR: Deprecate convert parameter in take
Browse files Browse the repository at this point in the history
xref gh-16948. The parameter is not respected,
nor is it a parameter in many 'take' implementations.
  • Loading branch information
gfyoung committed Sep 23, 2017
1 parent 4004367 commit 35812d0
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 73 deletions.
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v0.21.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ Other API Changes
Deprecations
~~~~~~~~~~~~
- :func:`read_excel()` has deprecated ``sheetname`` in favor of ``sheet_name`` for consistency with ``.to_excel()`` (:issue:`10559`).

- The ``convert`` parameter has been deprecated in the ``.take()`` method, as it was not being respected (:issue:`16948`)
- ``pd.options.html.border`` has been deprecated in favor of ``pd.options.display.html.border`` (:issue:`15793`).

- :func:`SeriesGroupBy.nth` has deprecated ``True`` in favor of ``'all'`` for its kwarg ``dropna`` (:issue:`11038`).
Expand Down
12 changes: 6 additions & 6 deletions pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -2032,7 +2032,7 @@ def _ixs(self, i, axis=0):
return self.loc[:, lab_slice]
else:
if isinstance(label, Index):
return self.take(i, axis=1, convert=True)
return self.take(i, axis=1)

index_len = len(self.index)

Expand Down Expand Up @@ -2114,10 +2114,10 @@ def _getitem_array(self, key):
# be reindexed to match DataFrame rows
key = check_bool_indexer(self.index, key)
indexer = key.nonzero()[0]
return self.take(indexer, axis=0, convert=False)
return self.take(indexer, axis=0)
else:
indexer = self.loc._convert_to_indexer(key, axis=1)
return self.take(indexer, axis=1, convert=True)
return self.take(indexer, axis=1)

def _getitem_multilevel(self, key):
loc = self.columns.get_loc(key)
Expand Down Expand Up @@ -3349,7 +3349,7 @@ def dropna(self, axis=0, how='any', thresh=None, subset=None,
else:
raise TypeError('must specify how or thresh')

result = self.take(mask.nonzero()[0], axis=axis, convert=False)
result = self.take(mask.nonzero()[0], axis=axis)

if inplace:
self._update_inplace(result)
Expand Down Expand Up @@ -3485,7 +3485,7 @@ def trans(v):

new_data = self._data.take(indexer,
axis=self._get_block_manager_axis(axis),
convert=False, verify=False)
verify=False)

if inplace:
return self._update_inplace(new_data)
Expand Down Expand Up @@ -3546,7 +3546,7 @@ def sort_index(self, axis=0, level=None, ascending=True, inplace=False,
baxis = self._get_block_manager_axis(axis)
new_data = self._data.take(indexer,
axis=baxis,
convert=False, verify=False)
verify=False)

# reconstruct axis if needed
new_data.axes[baxis] = new_data.axes[baxis]._sort_levels_monotonic()
Expand Down
95 changes: 76 additions & 19 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
from pandas.core.index import (Index, MultiIndex, _ensure_index,
InvalidIndexError)
import pandas.core.indexing as indexing
from pandas.core.indexing import maybe_convert_indices
from pandas.core.indexes.datetimes import DatetimeIndex
from pandas.core.indexes.period import PeriodIndex, Period
from pandas.core.internals import BlockManager
Expand Down Expand Up @@ -1820,7 +1821,7 @@ def _iget_item_cache(self, item):
if ax.is_unique:
lower = self._get_item_cache(ax[item])
else:
lower = self.take(item, axis=self._info_axis_number, convert=True)
lower = self.take(item, axis=self._info_axis_number)
return lower

def _box_item_values(self, key, values):
Expand Down Expand Up @@ -2055,8 +2056,63 @@ def __delitem__(self, key):
except KeyError:
pass

def take(self, indices, axis=0, convert=True, is_copy=True, **kwargs):
_shared_docs['_take'] = """
Return the elements in the given *positional* indices along an axis.
This means that we are not indexing according to actual values in
the index attribute of the object. We are indexing according to the
actual position of the element in the object.
This is the internal version of ``.take()`` and will contain a wider
selection of parameters useful for internal use but not as suitable
for public usage.
Parameters
----------
indices : array-like
An array of ints indicating which positions to take.
axis : int, default 0
The axis on which to select elements. "0" means that we are
selecting rows, "1" means that we are selecting columns, etc.
convert : bool, default True
Whether to convert negative indices into positive ones.
For example, ``-1`` would map to the ``len(axis) - 1``.
The conversions are similar to the behavior of indexing a
regular Python list.
is_copy : bool, default True
Whether to return a copy of the original object or not.
Returns
-------
taken : type of caller
An array-like containing the elements taken from the object.
See Also
--------
numpy.ndarray.take
numpy.take
"""

@Appender(_shared_docs['_take'])
def _take(self, indices, axis=0, convert=True, is_copy=False):
self._consolidate_inplace()

if convert:
indices = maybe_convert_indices(indices, len(self._get_axis(axis)))

new_data = self._data.take(indices,
axis=self._get_block_manager_axis(axis),
verify=True)
result = self._constructor(new_data).__finalize__(self)

# Maybe set copy if we didn't actually change the index.
if is_copy:
if not result._get_axis(axis).equals(self._get_axis(axis)):
result._set_is_copy(self)

return result

_shared_docs['take'] = """
Return the elements in the given *positional* indices along an axis.
This means that we are not indexing according to actual values in
Expand All @@ -2071,9 +2127,12 @@ def take(self, indices, axis=0, convert=True, is_copy=True, **kwargs):
The axis on which to select elements. "0" means that we are
selecting rows, "1" means that we are selecting columns, etc.
convert : bool, default True
Whether to convert negative indices to positive ones, just as with
indexing into Python lists. For example, if `-1` was passed in,
this index would be converted ``n - 1``.
.. deprecated:: 0.21.0
Whether to convert negative indices into positive ones.
For example, ``-1`` would map to the ``len(axis) - 1``.
The conversions are similar to the behavior of indexing a
regular Python list.
is_copy : bool, default True
Whether to return a copy of the original object or not.
Expand Down Expand Up @@ -2129,19 +2188,17 @@ class max_speed
numpy.ndarray.take
numpy.take
"""

@Appender(_shared_docs['take'])
def take(self, indices, axis=0, convert=True, is_copy=True, **kwargs):
nv.validate_take(tuple(), kwargs)
self._consolidate_inplace()
new_data = self._data.take(indices,
axis=self._get_block_manager_axis(axis),
convert=True, verify=True)
result = self._constructor(new_data).__finalize__(self)

# maybe set copy if we didn't actually change the index
if is_copy:
if not result._get_axis(axis).equals(self._get_axis(axis)):
result._set_is_copy(self)
if not convert:
msg = ("The 'convert' parameter is deprecated "
"and will be removed in a future version.")
warnings.warn(msg, FutureWarning, stacklevel=2)

return result
return self._take(indices, axis=axis, convert=convert, is_copy=is_copy)

def xs(self, key, axis=0, level=None, drop_level=True):
"""
Expand Down Expand Up @@ -2242,9 +2299,9 @@ def xs(self, key, axis=0, level=None, drop_level=True):
if isinstance(loc, np.ndarray):
if loc.dtype == np.bool_:
inds, = loc.nonzero()
return self.take(inds, axis=axis, convert=False)
return self.take(inds, axis=axis)
else:
return self.take(loc, axis=axis, convert=True)
return self.take(loc, axis=axis)

if not is_scalar(loc):
new_index = self.index[loc]
Expand Down Expand Up @@ -5057,7 +5114,7 @@ def at_time(self, time, asof=False):
"""
try:
indexer = self.index.indexer_at_time(time, asof=asof)
return self.take(indexer, convert=False)
return self.take(indexer)
except AttributeError:
raise TypeError('Index must be DatetimeIndex')

Expand All @@ -5081,7 +5138,7 @@ def between_time(self, start_time, end_time, include_start=True,
indexer = self.index.indexer_between_time(
start_time, end_time, include_start=include_start,
include_end=include_end)
return self.take(indexer, convert=False)
return self.take(indexer)
except AttributeError:
raise TypeError('Index must be DatetimeIndex')

Expand Down
8 changes: 4 additions & 4 deletions pandas/core/groupby.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ def _set_grouper(self, obj, sort=False):
indexer = self.indexer = ax.argsort(kind='mergesort')
ax = ax.take(indexer)
obj = obj.take(indexer, axis=self.axis,
convert=False, is_copy=False)
is_copy=False)

self.obj = obj
self.grouper = ax
Expand Down Expand Up @@ -643,7 +643,7 @@ def get_group(self, name, obj=None):
if not len(inds):
raise KeyError(name)

return obj.take(inds, axis=self.axis, convert=False)
return obj.take(inds, axis=self.axis)

def __iter__(self):
"""
Expand Down Expand Up @@ -2202,7 +2202,7 @@ def _aggregate_series_fast(self, obj, func):
# avoids object / Series creation overhead
dummy = obj._get_values(slice(None, 0)).to_dense()
indexer = get_group_index_sorter(group_index, ngroups)
obj = obj.take(indexer, convert=False).to_dense()
obj = obj.take(indexer).to_dense()
group_index = algorithms.take_nd(
group_index, indexer, allow_fill=False)
grouper = lib.SeriesGrouper(obj, func, group_index, ngroups,
Expand Down Expand Up @@ -4435,7 +4435,7 @@ def __iter__(self):
yield i, self._chop(sdata, slice(start, end))

def _get_sorted_data(self):
return self.data.take(self.sort_idx, axis=self.axis, convert=False)
return self.data._take(self.sort_idx, axis=self.axis, convert=False)

def _chop(self, sdata, slice_obj):
return sdata.iloc[slice_obj]
Expand Down
17 changes: 8 additions & 9 deletions pandas/core/indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ def _getitem_iterable(self, key, axis=0):
if is_bool_indexer(key):
key = check_bool_indexer(labels, key)
inds, = key.nonzero()
return self.obj.take(inds, axis=axis, convert=False)
return self.obj.take(inds, axis=axis)
else:
# Have the index compute an indexer or return None
# if it cannot handle; we only act on all found values
Expand Down Expand Up @@ -1126,15 +1126,14 @@ def _getitem_iterable(self, key, axis=0):
keyarr)

if new_indexer is not None:
result = self.obj.take(indexer[indexer != -1], axis=axis,
convert=False)
result = self.obj.take(indexer[indexer != -1], axis=axis)

result = result._reindex_with_indexers(
{axis: [new_target, new_indexer]},
copy=True, allow_dups=True)

else:
result = self.obj.take(indexer, axis=axis, convert=False)
result = self.obj.take(indexer, axis=axis)

return result

Expand Down Expand Up @@ -1265,7 +1264,7 @@ def _get_slice_axis(self, slice_obj, axis=0):
if isinstance(indexer, slice):
return self._slice(indexer, axis=axis, kind='iloc')
else:
return self.obj.take(indexer, axis=axis, convert=False)
return self.obj.take(indexer, axis=axis)


class _IXIndexer(_NDFrameIndexer):
Expand Down Expand Up @@ -1350,7 +1349,7 @@ def _getbool_axis(self, key, axis=0):
key = check_bool_indexer(labels, key)
inds, = key.nonzero()
try:
return self.obj.take(inds, axis=axis, convert=False)
return self.obj.take(inds, axis=axis)
except Exception as detail:
raise self._exception(detail)

Expand All @@ -1367,7 +1366,7 @@ def _get_slice_axis(self, slice_obj, axis=0):
if isinstance(indexer, slice):
return self._slice(indexer, axis=axis, kind='iloc')
else:
return self.obj.take(indexer, axis=axis, convert=False)
return self.obj.take(indexer, axis=axis)


class _LocIndexer(_LocationIndexer):
Expand Down Expand Up @@ -1707,7 +1706,7 @@ def _get_slice_axis(self, slice_obj, axis=0):
if isinstance(slice_obj, slice):
return self._slice(slice_obj, axis=axis, kind='iloc')
else:
return self.obj.take(slice_obj, axis=axis, convert=False)
return self.obj.take(slice_obj, axis=axis)

def _get_list_axis(self, key, axis=0):
"""
Expand All @@ -1723,7 +1722,7 @@ def _get_list_axis(self, key, axis=0):
Series object
"""
try:
return self.obj.take(key, axis=axis, convert=False)
return self.obj.take(key, axis=axis)
except IndexError:
# re-raise with different error message
raise IndexError("positional indexers are out-of-bounds")
Expand Down
35 changes: 12 additions & 23 deletions pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -2545,35 +2545,24 @@ def memory_usage(self, index=True, deep=False):
v += self.index.memory_usage(deep=deep)
return v

def take(self, indices, axis=0, convert=True, is_copy=False, **kwargs):
"""
return Series corresponding to requested indices
Parameters
----------
indices : list / array of ints
convert : translate negative to positive indices (default)
Returns
-------
taken : Series
See also
--------
numpy.ndarray.take
"""
if kwargs:
nv.validate_take(tuple(), kwargs)

# check/convert indicies here
@Appender(generic._shared_docs['_take'])
def _take(self, indices, axis=0, convert=True, is_copy=False):
if convert:
indices = maybe_convert_indices(indices, len(self._get_axis(axis)))

indices = _ensure_platform_int(indices)
new_index = self.index.take(indices)
new_values = self._values.take(indices)
return (self._constructor(new_values, index=new_index, fastpath=True)
.__finalize__(self))

result = (self._constructor(new_values, index=new_index,
fastpath=True).__finalize__(self))

# Maybe set copy if we didn't actually change the index.
if is_copy:
if not result._get_axis(axis).equals(self._get_axis(axis)):
result._set_is_copy(self)

return result

def isin(self, values):
"""
Expand Down
13 changes: 6 additions & 7 deletions pandas/core/sparse/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -602,16 +602,15 @@ def sparse_reindex(self, new_index):
sparse_index=new_index,
fill_value=self.fill_value).__finalize__(self)

@Appender(generic._shared_docs['take'])
def take(self, indices, axis=0, convert=True, *args, **kwargs):
"""
Sparse-compatible version of ndarray.take
convert = nv.validate_take_with_convert(convert, args, kwargs)

Returns
-------
taken : ndarray
"""
if not convert:
msg = ("The 'convert' parameter is deprecated "
"and will be removed in a future version.")
warnings.warn(msg, FutureWarning, stacklevel=2)

convert = nv.validate_take_with_convert(convert, args, kwargs)
new_values = SparseArray.take(self.values, indices)
new_index = self.index.take(indices)
return self._constructor(new_values,
Expand Down
Loading

0 comments on commit 35812d0

Please sign in to comment.