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

[REVIEW] Drop kind parameter from Index.get_slice_bound #12856

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: 1 addition & 2 deletions python/cudf/cudf/core/_base_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -1354,7 +1354,7 @@ def rename(self, name, inplace=False):
out.name = name
return out

def get_slice_bound(self, label, side, kind=None):
def get_slice_bound(self, label, side):
"""
Calculate slice bound that corresponds to given label.
Returns leftmost (one-past-the-rightmost if ``side=='right'``) position
Expand All @@ -1364,7 +1364,6 @@ def get_slice_bound(self, label, side, kind=None):
----------
label : object
side : {'left', 'right'}
kind : {'ix', 'loc', 'getitem'}

Returns
-------
Expand Down
12 changes: 2 additions & 10 deletions python/cudf/cudf/core/column/column.py
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,7 @@ def is_monotonic_decreasing(self) -> bool:
ascending=[False], null_position=None
)

def get_slice_bound(self, label: ScalarLike, side: str, kind: str) -> int:
def get_slice_bound(self, label: ScalarLike, side: str) -> int:
"""
Calculate slice bound that corresponds to given label.
Returns leftmost (one-past-the-rightmost if ``side=='right'``) position
Expand All @@ -893,22 +893,14 @@ def get_slice_bound(self, label: ScalarLike, side: str, kind: str) -> int:
----------
label : Scalar
side : {'left', 'right'}
kind : {'ix', 'loc', 'getitem'}
"""
if kind not in {"ix", "loc", "getitem", None}:
raise ValueError(
f"Invalid value for ``kind`` parameter,"
f" must be either one of the following: "
f"{'ix', 'loc', 'getitem', None}, but found: {kind}"
)

if side not in {"left", "right"}:
raise ValueError(
"Invalid value for side kwarg,"
" must be either 'left' or 'right': %s" % (side,)
)

# TODO: Handle errors/missing keys correctly
# Not currently using `kind` argument.
if side == "left":
return self.find_first_value(label, closest=True)
elif side == "right":
Expand Down
22 changes: 4 additions & 18 deletions python/cudf/cudf/core/index.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2018-2022, NVIDIA CORPORATION.
# Copyright (c) 2018-2023, NVIDIA CORPORATION.

from __future__ import annotations

Expand Down Expand Up @@ -510,7 +510,7 @@ def is_monotonic_decreasing(self):
return self._step < 0 or len(self) <= 1

@_cudf_nvtx_annotate
def get_slice_bound(self, label, side, kind=None):
def get_slice_bound(self, label, side):
"""
Calculate slice bound that corresponds to given label.
Returns leftmost (one-past-the-rightmost if ``side=='right'``) position
Expand All @@ -521,20 +521,12 @@ def get_slice_bound(self, label, side, kind=None):
label : int
A valid value in the ``RangeIndex``
side : {'left', 'right'}
kind : Unused
To keep consistency with other index types.

Returns
-------
int
Index of label.
"""
if kind is not None:
warnings.warn(
"'kind' argument in get_slice_bound is deprecated and will be "
"removed in a future version.",
FutureWarning,
)
if side not in {"left", "right"}:
raise ValueError(f"Unrecognized side parameter: {side}")

Expand Down Expand Up @@ -1413,14 +1405,8 @@ def notna(self):
notnull = notna

@_cudf_nvtx_annotate
def get_slice_bound(self, label, side, kind=None):
if kind is not None:
warnings.warn(
"'kind' argument in get_slice_bound is deprecated and will be "
"removed in a future version.",
FutureWarning,
)
return self._values.get_slice_bound(label, side, kind)
def get_slice_bound(self, label, side):
return self._values.get_slice_bound(label, side)

def is_numeric(self):
return False
Expand Down
46 changes: 16 additions & 30 deletions python/cudf/cudf/tests/test_monotonic.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2019-2022, NVIDIA CORPORATION.
# Copyright (c) 2019-2023, NVIDIA CORPORATION.

"""
Tests related to is_unique and is_monotonic attributes
Expand All @@ -16,7 +16,7 @@
RangeIndex,
StringIndex,
)
from cudf.testing._utils import assert_eq, expect_warning_if
from cudf.testing._utils import assert_eq


@pytest.mark.parametrize("testrange", [(10, 20, 1), (0, -10, -1), (5, 5, 1)])
Expand Down Expand Up @@ -261,15 +261,12 @@ def test_multiindex_tuples(testarr):
],
)
@pytest.mark.parametrize("side", ["left", "right"])
@pytest.mark.parametrize("kind", ["loc", "getitem", None])
def test_get_slice_bound(testlist, side, kind):
def test_get_slice_bound(testlist, side):
index = GenericIndex(testlist)
index_pd = pd.Index(testlist)
for label in testlist:
with pytest.warns(FutureWarning):
expect = index_pd.get_slice_bound(label, side, kind)
with expect_warning_if(kind is not None, FutureWarning):
got = index.get_slice_bound(label, side, kind)
expect = index_pd.get_slice_bound(label, side)
got = index.get_slice_bound(label, side)
assert got == expect


Expand All @@ -279,16 +276,13 @@ def test_get_slice_bound(testlist, side, kind):
[[-1, 0, 5, 10, 11], [-1, 0, 1, 2], [2, 3, 4, 5], [-1, 0, 1], [2, 3, 4]],
)
@pytest.mark.parametrize("side", ["left", "right"])
@pytest.mark.parametrize("kind", ["getitem", "loc"])
def test_rangeindex_get_slice_bound_basic(bounds, indices, side, kind):
def test_rangeindex_get_slice_bound_basic(bounds, indices, side):
start, stop = bounds
pd_index = pd.RangeIndex(start, stop)
cudf_index = RangeIndex(start, stop)
for idx in indices:
with pytest.warns(FutureWarning):
expect = pd_index.get_slice_bound(idx, side, kind)
with expect_warning_if(kind is not None, FutureWarning):
got = cudf_index.get_slice_bound(idx, side, kind)
expect = pd_index.get_slice_bound(idx, side)
got = cudf_index.get_slice_bound(idx, side)
assert expect == got


Expand All @@ -301,31 +295,25 @@ def test_rangeindex_get_slice_bound_basic(bounds, indices, side, kind):
[3, 8, 13, 18, 20, 15, 10, 5, -1, 0, 19, 21, 6, 11, 17],
)
@pytest.mark.parametrize("side", ["left", "right"])
@pytest.mark.parametrize("kind", ["getitem", "loc"])
def test_rangeindex_get_slice_bound_step(bounds, label, side, kind):
def test_rangeindex_get_slice_bound_step(bounds, label, side):
start, stop, step = bounds
pd_index = pd.RangeIndex(start, stop, step)
cudf_index = RangeIndex(start, stop, step)

with pytest.warns(FutureWarning):
expect = pd_index.get_slice_bound(label, side, kind)
with expect_warning_if(kind is not None, FutureWarning):
got = cudf_index.get_slice_bound(label, side, kind)
expect = pd_index.get_slice_bound(label, side)
got = cudf_index.get_slice_bound(label, side)
assert expect == got


@pytest.mark.parametrize("label", [1, 3, 5, 7, 9, 11])
@pytest.mark.parametrize("side", ["left", "right"])
@pytest.mark.parametrize("kind", ["loc", "getitem", None])
def test_get_slice_bound_missing(label, side, kind):
def test_get_slice_bound_missing(label, side):
mylist = [2, 4, 6, 8, 10]
index = GenericIndex(mylist)
index_pd = pd.Index(mylist)

with pytest.warns(FutureWarning):
expect = index_pd.get_slice_bound(label, side, kind)
with expect_warning_if(kind is not None, FutureWarning):
got = index.get_slice_bound(label, side, kind)
expect = index_pd.get_slice_bound(label, side)
got = index.get_slice_bound(label, side)
assert got == expect


Expand All @@ -338,10 +326,8 @@ def test_get_slice_bound_missing_str(label, side):
mylist = ["b", "d", "f"]
index = GenericIndex(mylist)
index_pd = pd.Index(mylist)
with pytest.warns(FutureWarning):
got = index.get_slice_bound(label, side, "getitem")
with pytest.warns(FutureWarning):
expect = index_pd.get_slice_bound(label, side, "getitem")
got = index.get_slice_bound(label, side)
expect = index_pd.get_slice_bound(label, side)
assert got == expect


Expand Down