From 7a26837b519b0e22e23171a9a545cbfa8907416c Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Sun, 2 Feb 2020 13:29:11 -0800 Subject: [PATCH 01/10] Allow step!=1 in slicing Series with IntervalIndex --- pandas/core/indexes/interval.py | 5 ----- pandas/tests/indexing/interval/test_interval_new.py | 12 ++++++++++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/pandas/core/indexes/interval.py b/pandas/core/indexes/interval.py index 6a3e808ab9821..ff34fd9665ca9 100644 --- a/pandas/core/indexes/interval.py +++ b/pandas/core/indexes/interval.py @@ -889,11 +889,6 @@ def get_value(self, series: "Series", key): loc = self.get_loc(key) return series.iloc[loc] - def _convert_slice_indexer(self, key: slice, kind=None): - if not (key.step is None or key.step == 1): - raise ValueError("cannot support not-default step in a slice") - return super()._convert_slice_indexer(key, kind) - @Appender(Index.where.__doc__) def where(self, cond, other=None): if other is None: diff --git a/pandas/tests/indexing/interval/test_interval_new.py b/pandas/tests/indexing/interval/test_interval_new.py index 43036fbbd9844..8201dd0907017 100644 --- a/pandas/tests/indexing/interval/test_interval_new.py +++ b/pandas/tests/indexing/interval/test_interval_new.py @@ -128,6 +128,7 @@ def test_loc_with_slices(self): with pytest.raises(NotImplementedError, match=msg): s[Interval(3, 4, closed="left") :] + # FIXME: dont leave commented-out # TODO with non-existing intervals ? # s.loc[Interval(-1, 0):Interval(2, 3)] @@ -143,9 +144,16 @@ def test_loc_with_slices(self): tm.assert_series_equal(expected, s[:2.5]) tm.assert_series_equal(expected, s[0.1:2.5]) + def test_slice_step_ne1(self): # slice of scalar with step != 1 - with pytest.raises(ValueError): - s[0:4:2] + s = self.s + expected = s.iloc[0:4:2] + + result = s[0:4:2] + tm.assert_series_equal(result, expected) + + result2 = s[0:4][::2] + tm.assert_series_equal(result2, expected) def test_loc_with_overlap(self): From f833f7cee5af023053635b36a923d0e7af67441b Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Tue, 4 Feb 2020 18:09:12 -0800 Subject: [PATCH 02/10] Whatsnew --- doc/source/whatsnew/v1.1.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index dc7edd8db662e..7d927a30a6d4a 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -145,7 +145,7 @@ Strings Interval ^^^^^^^^ - +- Bug in :class:`IntervalIndex` slicing that prevented slicing with ``step > 1`` (:issue:`31658`) - - From a2eb9bb962a048c55e0854fac0973fe6254f0205 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Wed, 5 Feb 2020 21:11:20 -0800 Subject: [PATCH 03/10] doc suggestion --- doc/source/whatsnew/v1.1.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index 862cfaa3226b8..0e7a751e67a4c 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -42,6 +42,7 @@ Other enhancements ^^^^^^^^^^^^^^^^^^ - :class:`Styler` may now render CSS more efficiently where multiple cells have the same styling (:issue:`30876`) +- Bug in :class:`IntervalIndex` slicing that prevented slicing with ``step > 1`` (:issue:`31658`) - - @@ -145,7 +146,6 @@ Strings Interval ^^^^^^^^ -- Bug in :class:`IntervalIndex` slicing that prevented slicing with ``step > 1`` (:issue:`31658`) - - From 6b97b09bbd3f601f91ae52fbb450c22244d576b3 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Wed, 5 Feb 2020 21:14:32 -0800 Subject: [PATCH 04/10] test for interval step --- pandas/tests/indexing/interval/test_interval_new.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pandas/tests/indexing/interval/test_interval_new.py b/pandas/tests/indexing/interval/test_interval_new.py index 8201dd0907017..7aa2292d32f2a 100644 --- a/pandas/tests/indexing/interval/test_interval_new.py +++ b/pandas/tests/indexing/interval/test_interval_new.py @@ -145,7 +145,7 @@ def test_loc_with_slices(self): tm.assert_series_equal(expected, s[0.1:2.5]) def test_slice_step_ne1(self): - # slice of scalar with step != 1 + # GH#31658 slice of scalar with step != 1 s = self.s expected = s.iloc[0:4:2] @@ -155,6 +155,17 @@ def test_slice_step_ne1(self): result2 = s[0:4][::2] tm.assert_series_equal(result2, expected) + def test_slice_interval_step(self): + # GH#31658 allows for integer step!=1, not Interval step + s = self.s + msg = ( + "cannot do slice indexing on " + " with " + "these indexers .* of " + ) + with pytest.raises(TypeError, match=msg): + s[0 : 4 : Interval(0, 1)] + def test_loc_with_overlap(self): idx = IntervalIndex.from_tuples([(1, 5), (3, 7)]) From a678ec21a2ef9b25066fb46ee241d77d2e773138 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 6 Feb 2020 09:11:48 -0800 Subject: [PATCH 05/10] remove commented-out --- pandas/tests/indexing/interval/test_interval_new.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pandas/tests/indexing/interval/test_interval_new.py b/pandas/tests/indexing/interval/test_interval_new.py index 7aa2292d32f2a..9be430c56cd4d 100644 --- a/pandas/tests/indexing/interval/test_interval_new.py +++ b/pandas/tests/indexing/interval/test_interval_new.py @@ -128,10 +128,6 @@ def test_loc_with_slices(self): with pytest.raises(NotImplementedError, match=msg): s[Interval(3, 4, closed="left") :] - # FIXME: dont leave commented-out - # TODO with non-existing intervals ? - # s.loc[Interval(-1, 0):Interval(2, 3)] - # slice of scalar expected = s.iloc[:3] From 2c684cc71b30023e8343bfb7b0dacec5b8d51f5a Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 6 Feb 2020 09:24:23 -0800 Subject: [PATCH 06/10] reword whatsnew --- doc/source/whatsnew/v1.1.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index 0e7a751e67a4c..f60cf368e0210 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -42,7 +42,7 @@ Other enhancements ^^^^^^^^^^^^^^^^^^ - :class:`Styler` may now render CSS more efficiently where multiple cells have the same styling (:issue:`30876`) -- Bug in :class:`IntervalIndex` slicing that prevented slicing with ``step > 1`` (:issue:`31658`) +- Positional slicing on a :class:`IntervalIndex` now supports slices with ``step > 1`` (:issue:`31658`) - - From 10165b4f9bd9e9a64e48b1663aaac4c030c322c0 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 6 Feb 2020 15:08:01 -0800 Subject: [PATCH 07/10] disallow label-based with step!=1 --- pandas/core/indexers.py | 29 +++++++++++++++++++ pandas/core/indexes/interval.py | 20 +++++++++++++ .../indexing/interval/test_interval_new.py | 17 +++++++---- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/pandas/core/indexers.py b/pandas/core/indexers.py index fe475527f4596..2ab7e77e316cd 100644 --- a/pandas/core/indexers.py +++ b/pandas/core/indexers.py @@ -10,6 +10,7 @@ from pandas.core.dtypes.common import ( is_array_like, is_bool_dtype, + is_integer, is_integer_dtype, is_list_like, ) @@ -19,6 +20,34 @@ # Indexer Identification +def is_valid_positional_slice(slc: slice) -> bool: + """ + Check if a slice object can be interpretd as a positional indexer. + + Parameters + ---------- + slc : slice + + Returns + ------- + bool + + Notes + ----- + A valid positional slice may also be interpreted as a label-based slice + depending on the index being sliced. + """ + + def is_int_or_none(val): + return val is None or is_integer(val) + + return ( + is_int_or_none(slc.start) + and is_int_or_none(slc.stop) + and is_int_or_none(slc.step) + ) + + def is_list_like_indexer(key) -> bool: """ Check if we have a list-like indexer that is *not* a NamedTuple. diff --git a/pandas/core/indexes/interval.py b/pandas/core/indexes/interval.py index 2d7b8b725712a..2409cdb324cc3 100644 --- a/pandas/core/indexes/interval.py +++ b/pandas/core/indexes/interval.py @@ -39,6 +39,7 @@ from pandas.core.algorithms import take_1d from pandas.core.arrays.interval import IntervalArray, _interval_shared_docs import pandas.core.common as com +from pandas.core.indexers import is_valid_positional_slice import pandas.core.indexes.base as ibase from pandas.core.indexes.base import ( Index, @@ -884,6 +885,25 @@ def get_indexer_for(self, target: AnyArrayLike, **kwargs) -> np.ndarray: return self.get_indexer_non_unique(target)[0] return self.get_indexer(target, **kwargs) + def _convert_slice_indexer(self, key: slice, kind=None): + def is_int_or_none(val): + return val is None or is_integer(val) + + if not (key.step is None or key.step == 1): + # GH#31658 if label-based, we require step == 1, + # if positional, we disallow float start/stop + msg = ( + "label-based slicing with step!=1 is not supported " "for IntervalIndex" + ) + if kind == "loc": + raise ValueError(msg) + elif kind == "getitem": + if not is_valid_positional_slice(key): + # i.e. this cannot be interpreted as a positional slice + raise ValueError(msg) + + return super()._convert_slice_indexer(key, kind) + @Appender(Index.where.__doc__) def where(self, cond, other=None): if other is None: diff --git a/pandas/tests/indexing/interval/test_interval_new.py b/pandas/tests/indexing/interval/test_interval_new.py index 9be430c56cd4d..03c3034772bc6 100644 --- a/pandas/tests/indexing/interval/test_interval_new.py +++ b/pandas/tests/indexing/interval/test_interval_new.py @@ -151,15 +151,20 @@ def test_slice_step_ne1(self): result2 = s[0:4][::2] tm.assert_series_equal(result2, expected) + def test_slice_float_start_stop(self): + # GH#31658 slicing with integers is positional, with floats is not + # supported + ser = Series(np.arange(5), IntervalIndex.from_breaks(np.arange(6))) + + msg = "label-based slicing with step!=1 is not supported for IntervalIndex" + with pytest.raises(ValueError, match=msg): + ser[1.5:9.5:2] + def test_slice_interval_step(self): # GH#31658 allows for integer step!=1, not Interval step s = self.s - msg = ( - "cannot do slice indexing on " - " with " - "these indexers .* of " - ) - with pytest.raises(TypeError, match=msg): + msg = "label-based slicing with step!=1 is not supported for IntervalIndex" + with pytest.raises(ValueError, match=msg): s[0 : 4 : Interval(0, 1)] def test_loc_with_overlap(self): From 8308eb880205b41f43b56078eebe0756bb4fd6bf Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 6 Feb 2020 15:32:09 -0800 Subject: [PATCH 08/10] black fixup --- pandas/core/indexes/interval.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pandas/core/indexes/interval.py b/pandas/core/indexes/interval.py index 2409cdb324cc3..e17bda19821d4 100644 --- a/pandas/core/indexes/interval.py +++ b/pandas/core/indexes/interval.py @@ -892,9 +892,7 @@ def is_int_or_none(val): if not (key.step is None or key.step == 1): # GH#31658 if label-based, we require step == 1, # if positional, we disallow float start/stop - msg = ( - "label-based slicing with step!=1 is not supported " "for IntervalIndex" - ) + msg = "label-based slicing with step!=1 is not supported for IntervalIndex" if kind == "loc": raise ValueError(msg) elif kind == "getitem": From cf1a393c4df95accd59285d53655f805f367177e Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Tue, 25 Feb 2020 12:09:42 -0800 Subject: [PATCH 09/10] update error message --- pandas/tests/indexes/test_base.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pandas/tests/indexes/test_base.py b/pandas/tests/indexes/test_base.py index 22f6af2af4aed..966f896159ad0 100644 --- a/pandas/tests/indexes/test_base.py +++ b/pandas/tests/indexes/test_base.py @@ -2620,7 +2620,8 @@ def test_convert_almost_null_slice(indices): key = slice(None, None, "foo") if isinstance(idx, pd.IntervalIndex): - with pytest.raises(ValueError, match="cannot support not-default step"): + msg = "label-based slicing with step!=1 is not supported for IntervalIndex" + with pytest.raises(ValueError, match=msg): idx._convert_slice_indexer(key, "loc") else: msg = "'>=' not supported between instances of 'str' and 'int'" From 2a698c0916e16624ca72762bbdff05e0cc60bd44 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Fri, 6 Mar 2020 16:58:26 -0800 Subject: [PATCH 10/10] typo fixup --- pandas/core/indexers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/indexers.py b/pandas/core/indexers.py index d3f0a7eb05569..71fd5b6aab821 100644 --- a/pandas/core/indexers.py +++ b/pandas/core/indexers.py @@ -23,7 +23,7 @@ def is_valid_positional_slice(slc: slice) -> bool: """ - Check if a slice object can be interpretd as a positional indexer. + Check if a slice object can be interpreted as a positional indexer. Parameters ----------