Skip to content

BUG/TST: fix several issues with slice bound checking code #6531

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

Merged
merged 1 commit into from
Mar 5, 2014
Merged
Show file tree
Hide file tree
Changes from all 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: 3 additions & 0 deletions doc/source/release.rst
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ API Changes
- Slicing and advanced/boolean indexing operations on ``Index`` classes will no
longer change type of the resulting index (:issue:`6440`).
- ``set_index`` no longer converts MultiIndexes to an Index of tuples (:issue:`6459`).
- Slicing with negative start, stop & step values handles corner cases better (:issue:`6531`):
- ``df.iloc[:-len(df)]`` is now empty
- ``df.iloc[len(df)::-1]`` now enumerates all elements in reverse

Experimental Features
~~~~~~~~~~~~~~~~~~~~~
Expand Down
5 changes: 0 additions & 5 deletions pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -1867,11 +1867,6 @@ def eval(self, expr, **kwargs):
kwargs['resolvers'] = kwargs.get('resolvers', ()) + resolvers
return _eval(expr, **kwargs)

def _slice(self, slobj, axis=0, raise_on_error=False, typ=None):
axis = self._get_block_manager_axis(axis)
new_data = self._data.get_slice(
slobj, axis=axis, raise_on_error=raise_on_error)
return self._constructor(new_data)

def _box_item_values(self, key, values):
items = self.columns[self.columns.get_loc(key)]
Expand Down
10 changes: 10 additions & 0 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,16 @@ def _clear_item_cache(self, i=None):
else:
self._item_cache.clear()

def _slice(self, slobj, axis=0, typ=None):
"""
Construct a slice of this container.

typ parameter is maintained for compatibility with Series slicing.

"""
axis = self._get_block_manager_axis(axis)
return self._constructor(self._data.get_slice(slobj, axis=axis))

def _set_item(self, key, value):
self._data.set(key, value)
self._clear_item_cache()
Expand Down
43 changes: 3 additions & 40 deletions pandas/core/indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,32 +91,8 @@ def _get_label(self, label, axis=0):
def _get_loc(self, key, axis=0):
return self.obj._ixs(key, axis=axis)

def _slice(self, obj, axis=0, raise_on_error=False, typ=None):

# make out-of-bounds into bounds of the object
if typ == 'iloc':
ax = self.obj._get_axis(axis)
l = len(ax)
start = obj.start
stop = obj.stop
step = obj.step
if start is not None:
# degenerate to return nothing
if start >= l:
return self._getitem_axis(tuple(),axis=axis)

# equiv to a null slice
elif start <= -l:
start = None
if stop is not None:
if stop > l:
stop = None
elif stop <= -l:
stop = None
obj = slice(start,stop,step)

return self.obj._slice(obj, axis=axis, raise_on_error=raise_on_error,
typ=typ)
def _slice(self, obj, axis=0, typ=None):
return self.obj._slice(obj, axis=axis, typ=typ)

def __setitem__(self, key, value):

Expand Down Expand Up @@ -1343,8 +1319,7 @@ def _get_slice_axis(self, slice_obj, axis=0):
return obj

if isinstance(slice_obj, slice):
return self._slice(slice_obj, axis=axis, raise_on_error=True,
typ='iloc')
return self._slice(slice_obj, axis=axis, typ='iloc')
else:
return self.obj.take(slice_obj, axis=axis, convert=False)

Expand Down Expand Up @@ -1647,18 +1622,6 @@ def _need_slice(obj):
(obj.step is not None and obj.step != 1))


def _check_slice_bounds(slobj, values):
l = len(values)
start = slobj.start
if start is not None:
if start < -l or start > l - 1:
raise IndexError("out-of-bounds on slice (start)")
stop = slobj.stop
if stop is not None:
if stop < -l - 1 or stop > l:
raise IndexError("out-of-bounds on slice (end)")


def _maybe_droplevels(index, key):
# drop levels
original_index = index
Expand Down
12 changes: 3 additions & 9 deletions pandas/core/internals.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
_values_from_object, _is_null_datelike_scalar)
from pandas.core.index import (Index, MultiIndex, _ensure_index,
_handle_legacy_indexes)
from pandas.core.indexing import (_check_slice_bounds, _maybe_convert_indices,
_length_of_indexer)
from pandas.core.indexing import (_maybe_convert_indices, _length_of_indexer)
import pandas.core.common as com
from pandas.sparse.array import _maybe_to_sparse, SparseArray
import pandas.lib as lib
Expand Down Expand Up @@ -2669,12 +2668,9 @@ def combine(self, blocks):
new_axes[0] = new_items
return self.__class__(new_blocks, new_axes, do_integrity_check=False)

def get_slice(self, slobj, axis=0, raise_on_error=False):
def get_slice(self, slobj, axis=0):
new_axes = list(self.axes)

if raise_on_error:
_check_slice_bounds(slobj, new_axes[axis])

new_axes[axis] = new_axes[axis][slobj]

if axis == 0:
Expand Down Expand Up @@ -3739,9 +3735,7 @@ def _delete_from_block(self, i, item):
)
self._values = self._block.values

def get_slice(self, slobj, raise_on_error=False):
if raise_on_error:
_check_slice_bounds(slobj, self.index)
def get_slice(self, slobj):
return self.__class__(self._block._slice(slobj),
self.index[slobj], fastpath=True)

Expand Down
6 changes: 0 additions & 6 deletions pandas/core/panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,12 +539,6 @@ def _box_item_values(self, key, values):
d = self._construct_axes_dict_for_slice(self._AXIS_ORDERS[1:])
return self._constructor_sliced(values, **d)

def _slice(self, slobj, axis=0, raise_on_error=False, typ=None):
new_data = self._data.get_slice(slobj,
axis=axis,
raise_on_error=raise_on_error)
return self._constructor(new_data)

def __setitem__(self, key, value):
shape = tuple(self.shape)
if isinstance(value, self._constructor_sliced):
Expand Down
6 changes: 2 additions & 4 deletions pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from pandas.core.index import (Index, MultiIndex, InvalidIndexError,
_ensure_index, _handle_legacy_indexes)
from pandas.core.indexing import (
_check_bool_indexer, _check_slice_bounds,
_check_bool_indexer,
_is_index_slice, _maybe_convert_indices)
from pandas.core import generic, base
from pandas.core.internals import SingleBlockManager
Expand Down Expand Up @@ -469,9 +469,7 @@ def _ixs(self, i, axis=0):
def _is_mixed_type(self):
return False

def _slice(self, slobj, axis=0, raise_on_error=False, typ=None):
if raise_on_error:
_check_slice_bounds(slobj, self.values)
def _slice(self, slobj, axis=0, typ=None):
slobj = self.index._convert_slice_indexer(slobj, typ=typ or 'getitem')
return self._get_values(slobj)

Expand Down
8 changes: 2 additions & 6 deletions pandas/sparse/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from pandas.core.common import (isnull, notnull, _pickle_array,
_unpickle_array, _try_sort)
from pandas.core.index import Index, MultiIndex, _ensure_index
from pandas.core.indexing import _check_slice_bounds, _maybe_convert_indices
from pandas.core.indexing import _maybe_convert_indices
from pandas.core.series import Series
from pandas.core.frame import (DataFrame, extract_index, _prep_ndarray,
_default_index)
Expand Down Expand Up @@ -379,15 +379,11 @@ def set_value(self, index, col, value, takeable=False):
return dense.to_sparse(kind=self._default_kind,
fill_value=self._default_fill_value)

def _slice(self, slobj, axis=0, raise_on_error=False, typ=None):
def _slice(self, slobj, axis=0, typ=None):
if axis == 0:
if raise_on_error:
_check_slice_bounds(slobj, self.index)
new_index = self.index[slobj]
new_columns = self.columns
else:
if raise_on_error:
_check_slice_bounds(slobj, self.columns)
new_index = self.index
new_columns = self.columns[slobj]

Expand Down
2 changes: 1 addition & 1 deletion pandas/sparse/panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ def _ixs(self, i, axis=0):

return self.xs(key, axis=axis)

def _slice(self, slobj, axis=0, raise_on_error=False, typ=None):
def _slice(self, slobj, axis=0, typ=None):
"""
for compat as we don't support Block Manager here
"""
Expand Down
38 changes: 35 additions & 3 deletions pandas/tests/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,19 +393,51 @@ def test_iloc_exceeds_bounds(self):
self.assertRaises(IndexError, lambda : df.iloc[-30])

# slices are ok
result = df.iloc[:,4:10]
result = df.iloc[:,4:10] # 0 < start < len < stop
expected = df.iloc[:,4:]
assert_frame_equal(result,expected)

result = df.iloc[:,-4:-10]
expected = df.iloc[:,-4:]
result = df.iloc[:,-4:-10] # stop < 0 < start < len
expected = df.iloc[:,:0]
assert_frame_equal(result,expected)

result = df.iloc[:,10:4:-1] # 0 < stop < len < start (down)
expected = df.iloc[:,:4:-1]
assert_frame_equal(result,expected)

result = df.iloc[:,4:-10:-1] # stop < 0 < start < len (down)
expected = df.iloc[:,4::-1]
assert_frame_equal(result,expected)

result = df.iloc[:,-10:4] # start < 0 < stop < len
expected = df.iloc[:,:4]
assert_frame_equal(result,expected)

result = df.iloc[:,10:4] # 0 < stop < len < start
expected = df.iloc[:,:0]
assert_frame_equal(result,expected)

result = df.iloc[:,-10:-11:-1] # stop < start < 0 < len (down)
expected = df.iloc[:,:0]
assert_frame_equal(result,expected)

result = df.iloc[:,10:11] # 0 < len < start < stop
expected = df.iloc[:,:0]
assert_frame_equal(result,expected)

# slice bounds exceeding is ok
result = s.iloc[18:30]
expected = s.iloc[18:]
assert_series_equal(result,expected)

result = s.iloc[30:]
expected = s.iloc[:0]
assert_series_equal(result,expected)

result = s.iloc[30::-1]
expected = s.iloc[::-1]
assert_series_equal(result,expected)

# doc example
def check(result,expected):
str(result)
Expand Down