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: kind kwarg in _maybe_cast_slice_bound #41378

Merged
merged 2 commits into from
May 10, 2021
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
1 change: 1 addition & 0 deletions doc/source/user_guide/scale.rst
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ we need to supply the divisions manually.
Now we can do things like fast random access with ``.loc``.

.. ipython:: python
:okwarning:

ddf.loc["2002-01-01 12:01":"2002-01-01 12:05"].compute()

Expand Down
29 changes: 21 additions & 8 deletions pandas/core/indexes/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3685,7 +3685,7 @@ def is_int(v):
)
indexer = key
else:
indexer = self.slice_indexer(start, stop, step, kind=kind)
indexer = self.slice_indexer(start, stop, step)
Copy link

Choose a reason for hiding this comment

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

Hi, would this change possibly introduce this regression (we have pandas-1.3.0)?

14:18:02  /opt/conda/envs/birdy/lib/python3.7/site-packages/pandas/core/indexes/base.py in slice_indexer(self, start, end, step, kind)
14:18:02     5684         slice(1, 3, None)
14:18:02     5685         """
14:18:02  -> 5686         start_slice, end_slice = self.slice_locs(start, end, step=step)
14:18:02     5687 
14:18:02     5688         # return a slice
14:18:02  
14:18:02  /opt/conda/envs/birdy/lib/python3.7/site-packages/pandas/core/indexes/base.py in slice_locs(self, start, end, step, kind)
14:18:02     5886         start_slice = None
14:18:02     5887         if start is not None:
14:18:02  -> 5888             start_slice = self.get_slice_bound(start, "left")
14:18:02     5889         if start_slice is None:
14:18:02     5890             start_slice = 0
14:18:02  
14:18:02  /opt/conda/envs/birdy/lib/python3.7/site-packages/pandas/core/indexes/base.py in get_slice_bound(self, label, side, kind)
14:18:02     5796         # For datetime indices label may be a string that has to be converted
14:18:02     5797         # to datetime boundary according to its resolution.
14:18:02  -> 5798         label = self._maybe_cast_slice_bound(label, side)
14:18:02     5799 
14:18:02     5800         # we need to look up the label
14:18:02  
14:18:02  TypeError: _maybe_cast_slice_bound() missing 1 required positional argument: 'kind'

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you sure that traceback is produced using 1.3.0? 'kind' is a keyword argument with a default value, so i dont see how that traceback could be produced

Copy link

Choose a reason for hiding this comment

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

@jbrockmendel yes it is pandas-1.3.0 from our docker image https://hub.docker.com/repository/docker/pavics/workflow-tests tag preview.

$ docker run -it --rm --entrypoint bash pavics/workflow-tests:preview

$ conda init bash
$ bash
$ conda activate birdy
$ conda env export |grep pandas
  - geopandas=0.9.0=pyhd8ed1ab_1  
  - geopandas-base=0.9.0=pyhd8ed1ab_1                      
  - intake-geopandas=0.2.4=pyhd8ed1ab_0                                
  - pandas=1.3.0=py37h219a48f_0

This is the full error from our notebook https://github.com/bird-house/finch/blob/0346b3aa5096b88ef5bf03339a8c67ca7e4d6fdd/docs/source/notebooks/dap_subset.ipynb

18:18:02  _________ finch-master/docs/source/notebooks/dap_subset.ipynb::Cell 2 __________
18:18:02  Notebook cell execution failed
18:18:02  Cell 2: Cell execution caused an exception
18:18:02  
18:18:02  Input:
18:18:02  #NBVAL_IGNORE_OUTPUT
18:18:02  # Use the `remap_label_indexers` function to convert coordinates to *positional* indexes.
18:18:02  import datetime as dt
18:18:02  coords = dict(lat=45, lon=290)
18:18:02  index, _ = xr.core.coordinates.remap_label_indexers(ds, coords, method="nearest")
18:18:02  
18:18:02  # The `nearest` method cannot be used with slices, so we do another call for the time period.
18:18:02  ti, _ = xr.core.coordinates.remap_label_indexers(ds, dict(time=slice("2060-01-01", "2064-12-30")))
18:18:02  
18:18:02  # Merge the spatial and temporal indices
18:18:02  index.update(ti)
18:18:02  index
18:18:02  
18:18:02  Traceback:
18:18:02  
18:18:02  ---------------------------------------------------------------------------
18:18:02  TypeError                                 Traceback (most recent call last)
18:18:02  <ipython-input-3-d0a5be34f657> in <module>
18:18:02        6 
18:18:02        7 # The `nearest` method cannot be used with slices, so we do another call for the time period.
18:18:02  ----> 8 ti, _ = xr.core.coordinates.remap_label_indexers(ds, dict(time=slice("2060-01-01", "2064-12-30")))
18:18:02        9 
18:18:02       10 # Merge the spatial and temporal indices
18:18:02  
18:18:02  /opt/conda/envs/birdy/lib/python3.7/site-packages/xarray/core/coordinates.py in remap_label_indexers(obj, indexers, method, tolerance, **indexers_kwargs)
18:18:02      420 
18:18:02      421     pos_indexers, new_indexes = indexing.remap_label_indexers(
18:18:02  --> 422         obj, v_indexers, method=method, tolerance=tolerance
18:18:02      423     )
18:18:02      424     # attach indexer's coordinate to pos_indexers
18:18:02  
18:18:02  /opt/conda/envs/birdy/lib/python3.7/site-packages/xarray/core/indexing.py in remap_label_indexers(data_obj, indexers, method, tolerance)
18:18:02      272             coords_dtype = data_obj.coords[dim].dtype
18:18:02      273             label = maybe_cast_to_coords_dtype(label, coords_dtype)
18:18:02  --> 274             idxr, new_idx = convert_label_indexer(index, label, dim, method, tolerance)
18:18:02      275             pos_indexers[dim] = idxr
18:18:02      276             if new_idx is not None:
18:18:02  
18:18:02  /opt/conda/envs/birdy/lib/python3.7/site-packages/xarray/core/indexing.py in convert_label_indexer(index, label, index_name, method, tolerance)
18:18:02      122             _sanitize_slice_element(label.start),
18:18:02      123             _sanitize_slice_element(label.stop),
18:18:02  --> 124             _sanitize_slice_element(label.step),
18:18:02      125         )
18:18:02      126         if not isinstance(indexer, slice):
18:18:02  
18:18:02  /opt/conda/envs/birdy/lib/python3.7/site-packages/pandas/core/indexes/base.py in slice_indexer(self, start, end, step, kind)
18:18:02     5684         slice(1, 3, None)
18:18:02     5685         """
18:18:02  -> 5686         start_slice, end_slice = self.slice_locs(start, end, step=step)
18:18:02     5687 
18:18:02     5688         # return a slice
18:18:02  
18:18:02  /opt/conda/envs/birdy/lib/python3.7/site-packages/pandas/core/indexes/base.py in slice_locs(self, start, end, step, kind)
18:18:02     5886         start_slice = None
18:18:02     5887         if start is not None:
18:18:02  -> 5888             start_slice = self.get_slice_bound(start, "left")
18:18:02     5889         if start_slice is None:
18:18:02     5890             start_slice = 0
18:18:02  
18:18:02  /opt/conda/envs/birdy/lib/python3.7/site-packages/pandas/core/indexes/base.py in get_slice_bound(self, label, side, kind)
18:18:02     5796         # For datetime indices label may be a string that has to be converted
18:18:02     5797         # to datetime boundary according to its resolution.
18:18:02  -> 5798         label = self._maybe_cast_slice_bound(label, side)
18:18:02     5799 
18:18:02     5800         # we need to look up the label
18:18:02  
18:18:02  TypeError: _maybe_cast_slice_bound() missing 1 required positional argument: 'kind'

Copy link
Member Author

Choose a reason for hiding this comment

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

What kind of Index do you have here? maybe its a custom subclass that overrides _maybe_cast_slice_bound and has kind as a positional argument? All of our Index subclasses have kind as a keyword argument

Copy link

Choose a reason for hiding this comment

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

What kind of Index do you have here? maybe its a custom subclass that overrides _maybe_cast_slice_bound and has kind as a positional argument? All of our Index subclasses have kind as a keyword argument

You are right, xarray has an override !

$ grep -R  _maybe_cast_slice_bound . | grep def
./lib/python3.7/site-packages/xarray/coding/cftimeindex.py:    def _maybe_cast_slice_bound(self, label, side, kind):

And they seem to already made the matching change as well pydata/xarray#5359 but did not cut a release containing the change yet !


return indexer

Expand Down Expand Up @@ -5648,7 +5648,7 @@ def slice_indexer(
>>> idx.slice_indexer(start='b', end=('c', 'g'))
slice(1, 3, None)
"""
start_slice, end_slice = self.slice_locs(start, end, step=step, kind=kind)
start_slice, end_slice = self.slice_locs(start, end, step=step)

# return a slice
if not is_scalar(start_slice):
Expand Down Expand Up @@ -5678,7 +5678,7 @@ def _validate_indexer(self, form: str_t, key, kind: str_t):
if key is not None and not is_integer(key):
raise self._invalid_indexer(form, key)

def _maybe_cast_slice_bound(self, label, side: str_t, kind):
def _maybe_cast_slice_bound(self, label, side: str_t, kind=no_default):
"""
This function should be overloaded in subclasses that allow non-trivial
casting on label-slice bounds, e.g. datetime-like indices allowing
Expand All @@ -5698,7 +5698,8 @@ def _maybe_cast_slice_bound(self, label, side: str_t, kind):
-----
Value of `side` parameter should be validated in caller.
"""
assert kind in ["loc", "getitem", None]
assert kind in ["loc", "getitem", None, no_default]
self._deprecated_arg(kind, "kind", "_maybe_cast_slice_bound")

# We are a plain index here (sub-class override this method if they
# wish to have special treatment for floats/ints, e.g. Float64Index and
Expand All @@ -5723,7 +5724,7 @@ def _searchsorted_monotonic(self, label, side: str_t = "left"):

raise ValueError("index must be monotonic increasing or decreasing")

def get_slice_bound(self, label, side: str_t, kind) -> int:
def get_slice_bound(self, label, side: str_t, kind=None) -> int:
"""
Calculate slice bound that corresponds to given label.

Expand Down Expand Up @@ -5753,7 +5754,7 @@ def get_slice_bound(self, label, side: str_t, kind) -> int:

# For datetime indices label may be a string that has to be converted
# to datetime boundary according to its resolution.
label = self._maybe_cast_slice_bound(label, side, kind)
label = self._maybe_cast_slice_bound(label, side)

# we need to look up the label
try:
Expand Down Expand Up @@ -5843,13 +5844,13 @@ def slice_locs(self, start=None, end=None, step=None, kind=None):

start_slice = None
if start is not None:
start_slice = self.get_slice_bound(start, "left", kind)
start_slice = self.get_slice_bound(start, "left")
if start_slice is None:
start_slice = 0

end_slice = None
if end is not None:
end_slice = self.get_slice_bound(end, "right", kind)
end_slice = self.get_slice_bound(end, "right")
if end_slice is None:
end_slice = len(self)

Expand Down Expand Up @@ -6181,6 +6182,18 @@ def shape(self) -> Shape:
# See GH#27775, GH#27384 for history/reasoning in how this is defined.
return (len(self),)

def _deprecated_arg(self, value, name: str_t, methodname: str_t) -> None:
"""
Issue a FutureWarning if the arg/kwarg is not no_default.
"""
if value is not no_default:
warnings.warn(
f"'{name}' argument in {methodname} is deprecated "
"and will be removed in a future version. Do not pass it.",
FutureWarning,
stacklevel=3,
)


def ensure_index_from_sequences(sequences, names=None):
"""
Expand Down
9 changes: 5 additions & 4 deletions pandas/core/indexes/datetimes.py
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ def _maybe_cast_for_get_loc(self, key) -> Timestamp:
key = key.tz_convert(self.tz)
return key

def _maybe_cast_slice_bound(self, label, side: str, kind):
def _maybe_cast_slice_bound(self, label, side: str, kind=lib.no_default):
"""
If label is a string, cast it to datetime according to resolution.

Expand All @@ -742,7 +742,8 @@ def _maybe_cast_slice_bound(self, label, side: str, kind):
-----
Value of `side` parameter should be validated in caller.
"""
assert kind in ["loc", "getitem", None]
assert kind in ["loc", "getitem", None, lib.no_default]
self._deprecated_arg(kind, "kind", "_maybe_cast_slice_bound")

if isinstance(label, str):
freq = getattr(self, "freqstr", getattr(self, "inferred_freq", None))
Expand Down Expand Up @@ -823,12 +824,12 @@ def check_str_or_none(point):
mask = np.array(True)
deprecation_mask = np.array(True)
if start is not None:
start_casted = self._maybe_cast_slice_bound(start, "left", kind)
start_casted = self._maybe_cast_slice_bound(start, "left")
mask = start_casted <= self
deprecation_mask = start_casted == self

if end is not None:
end_casted = self._maybe_cast_slice_bound(end, "right", kind)
end_casted = self._maybe_cast_slice_bound(end, "right")
mask = (self <= end_casted) & mask
deprecation_mask = (end_casted == self) | deprecation_mask

Expand Down
5 changes: 3 additions & 2 deletions pandas/core/indexes/interval.py
Original file line number Diff line number Diff line change
Expand Up @@ -825,8 +825,9 @@ def _should_fallback_to_positional(self) -> bool:
# positional in this case
return self.dtype.subtype.kind in ["m", "M"]

def _maybe_cast_slice_bound(self, label, side: str, kind):
return getattr(self, side)._maybe_cast_slice_bound(label, side, kind)
def _maybe_cast_slice_bound(self, label, side: str, kind=lib.no_default):
self._deprecated_arg(kind, "kind", "_maybe_cast_slice_bound")
return getattr(self, side)._maybe_cast_slice_bound(label, side)

@Appender(Index._convert_list_indexer.__doc__)
def _convert_list_indexer(self, keyarr):
Expand Down
14 changes: 6 additions & 8 deletions pandas/core/indexes/multi.py
Original file line number Diff line number Diff line change
Expand Up @@ -2716,7 +2716,7 @@ def _get_indexer(
return ensure_platform_int(indexer)

def get_slice_bound(
self, label: Hashable | Sequence[Hashable], side: str, kind: str
self, label: Hashable | Sequence[Hashable], side: str, kind: str | None = None
) -> int:
"""
For an ordered MultiIndex, compute slice bound
Expand All @@ -2729,7 +2729,7 @@ def get_slice_bound(
----------
label : object or tuple of objects
side : {'left', 'right'}
kind : {'loc', 'getitem'}
kind : {'loc', 'getitem', None}

Returns
-------
Expand All @@ -2747,13 +2747,13 @@ def get_slice_bound(
Get the locations from the leftmost 'b' in the first level
until the end of the multiindex:

>>> mi.get_slice_bound('b', side="left", kind="loc")
>>> mi.get_slice_bound('b', side="left")
1

Like above, but if you get the locations from the rightmost
'b' in the first level and 'f' in the second level:

>>> mi.get_slice_bound(('b','f'), side="right", kind="loc")
>>> mi.get_slice_bound(('b','f'), side="right")
3

See Also
Expand Down Expand Up @@ -2820,7 +2820,7 @@ def slice_locs(self, start=None, end=None, step=None, kind=None):
"""
# This function adds nothing to its parent implementation (the magic
# happens in get_slice_bound method), but it adds meaningful doc.
return super().slice_locs(start, end, step, kind=kind)
return super().slice_locs(start, end, step)

def _partial_tup_index(self, tup, side="left"):
if len(tup) > self._lexsort_depth:
Expand Down Expand Up @@ -3206,9 +3206,7 @@ def convert_indexer(start, stop, step, indexer=indexer, codes=level_codes):

# we have a partial slice (like looking up a partial date
# string)
start = stop = level_index.slice_indexer(
key.start, key.stop, key.step, kind="loc"
)
start = stop = level_index.slice_indexer(key.start, key.stop, key.step)
step = start.step

if isinstance(start, slice) or isinstance(stop, slice):
Expand Down
7 changes: 4 additions & 3 deletions pandas/core/indexes/numeric.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ def _validate_dtype(cls, dtype: Dtype | None) -> None:
# Indexing Methods

@doc(Index._maybe_cast_slice_bound)
def _maybe_cast_slice_bound(self, label, side: str, kind):
assert kind in ["loc", "getitem", None]
def _maybe_cast_slice_bound(self, label, side: str, kind=lib.no_default):
assert kind in ["loc", "getitem", None, lib.no_default]
self._deprecated_arg(kind, "kind", "_maybe_cast_slice_bound")

# we will try to coerce to integers
return self._maybe_cast_indexer(label)
Expand Down Expand Up @@ -346,7 +347,7 @@ def _convert_slice_indexer(self, key: slice, kind: str):

# We always treat __getitem__ slicing as label-based
# translate to locations
return self.slice_indexer(key.start, key.stop, key.step, kind=kind)
return self.slice_indexer(key.start, key.stop, key.step)

# ----------------------------------------------------------------

Expand Down
7 changes: 4 additions & 3 deletions pandas/core/indexes/period.py
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ def get_loc(self, key, method=None, tolerance=None):
except KeyError as err:
raise KeyError(orig_key) from err

def _maybe_cast_slice_bound(self, label, side: str, kind: str):
def _maybe_cast_slice_bound(self, label, side: str, kind=lib.no_default):
"""
If label is a string or a datetime, cast it to Period.ordinal according
to resolution.
Expand All @@ -540,7 +540,7 @@ def _maybe_cast_slice_bound(self, label, side: str, kind: str):
----------
label : object
side : {'left', 'right'}
kind : {'loc', 'getitem'}
kind : {'loc', 'getitem'}, or None

Returns
-------
Expand All @@ -551,7 +551,8 @@ def _maybe_cast_slice_bound(self, label, side: str, kind: str):
Value of `side` parameter should be validated in caller.

"""
assert kind in ["loc", "getitem"]
assert kind in ["loc", "getitem", None, lib.no_default]
self._deprecated_arg(kind, "kind", "_maybe_cast_slice_bound")

if isinstance(label, datetime):
return Period(label, freq=self.freq)
Expand Down
5 changes: 3 additions & 2 deletions pandas/core/indexes/timedeltas.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ def get_loc(self, key, method=None, tolerance=None):

return Index.get_loc(self, key, method, tolerance)

def _maybe_cast_slice_bound(self, label, side: str, kind):
def _maybe_cast_slice_bound(self, label, side: str, kind=lib.no_default):
"""
If label is a string, cast it to timedelta according to resolution.

Expand All @@ -206,7 +206,8 @@ def _maybe_cast_slice_bound(self, label, side: str, kind):
-------
label : object
"""
assert kind in ["loc", "getitem", None]
assert kind in ["loc", "getitem", None, lib.no_default]
self._deprecated_arg(kind, "kind", "_maybe_cast_slice_bound")

if isinstance(label, str):
parsed = Timedelta(label)
Expand Down
4 changes: 1 addition & 3 deletions pandas/core/indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1170,9 +1170,7 @@ def _get_slice_axis(self, slice_obj: slice, axis: int):
return obj.copy(deep=False)

labels = obj._get_axis(axis)
indexer = labels.slice_indexer(
slice_obj.start, slice_obj.stop, slice_obj.step, kind="loc"
)
indexer = labels.slice_indexer(slice_obj.start, slice_obj.stop, slice_obj.step)

if isinstance(indexer, slice):
return self.obj._slice(indexer, axis=axis)
Expand Down
6 changes: 3 additions & 3 deletions pandas/tests/indexes/datetimes/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -679,18 +679,18 @@ def test_maybe_cast_slice_bounds_empty(self):
# GH#14354
empty_idx = date_range(freq="1H", periods=0, end="2015")

right = empty_idx._maybe_cast_slice_bound("2015-01-02", "right", "loc")
right = empty_idx._maybe_cast_slice_bound("2015-01-02", "right")
exp = Timestamp("2015-01-02 23:59:59.999999999")
assert right == exp

left = empty_idx._maybe_cast_slice_bound("2015-01-02", "left", "loc")
left = empty_idx._maybe_cast_slice_bound("2015-01-02", "left")
exp = Timestamp("2015-01-02 00:00:00")
assert left == exp

def test_maybe_cast_slice_duplicate_monotonic(self):
# https://github.com/pandas-dev/pandas/issues/16515
idx = DatetimeIndex(["2017", "2017"])
result = idx._maybe_cast_slice_bound("2017-01-01", "left", "loc")
result = idx._maybe_cast_slice_bound("2017-01-01", "left")
expected = Timestamp("2017-01-01")
assert result == expected

Expand Down
4 changes: 2 additions & 2 deletions pandas/tests/indexes/period/test_partial_slicing.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,9 @@ def test_maybe_cast_slice_bound(self, make_range, frame_or_series):

# Check the lower-level calls are raising where expected.
with pytest.raises(TypeError, match=msg):
idx._maybe_cast_slice_bound("foo", "left", "loc")
idx._maybe_cast_slice_bound("foo", "left")
with pytest.raises(TypeError, match=msg):
idx.get_slice_bound("foo", "left", "loc")
idx.get_slice_bound("foo", "left")

with pytest.raises(TypeError, match=msg):
obj["2013/09/30":"foo"]
Expand Down
13 changes: 13 additions & 0 deletions pandas/tests/indexes/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,3 +259,16 @@ def test_getitem_deprecated_float(idx):

expected = idx[1]
assert result == expected


def test_maybe_cast_slice_bound_kind_deprecated(index):
if not len(index):
return

with tm.assert_produces_warning(FutureWarning):
# passed as keyword
index._maybe_cast_slice_bound(index[0], "left", kind="loc")

with tm.assert_produces_warning(FutureWarning):
# pass as positional
index._maybe_cast_slice_bound(index[0], "left", "loc")