From 5fc8ceba880030654b072bd163b1f2b1e8b7de78 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Sat, 18 Jan 2020 18:01:22 -0800 Subject: [PATCH 1/2] REF/BUG: get_value called incorrectly, de-duplicate+simplify --- pandas/core/indexes/base.py | 59 ++++++++----------- pandas/core/indexes/datetimes.py | 5 +- pandas/core/indexes/timedeltas.py | 4 +- .../tests/indexes/datetimes/test_indexing.py | 12 ++-- pandas/tests/indexes/test_base.py | 7 ++- 5 files changed, 43 insertions(+), 44 deletions(-) diff --git a/pandas/core/indexes/base.py b/pandas/core/indexes/base.py index 5ce2b06ed7dbd..030cda84dfebe 100644 --- a/pandas/core/indexes/base.py +++ b/pandas/core/indexes/base.py @@ -68,7 +68,6 @@ from pandas.core.arrays import ExtensionArray from pandas.core.base import IndexOpsMixin, PandasObject import pandas.core.common as com -from pandas.core.construction import extract_array from pandas.core.indexers import maybe_convert_indices from pandas.core.indexes.frozen import FrozenList import pandas.core.missing as missing @@ -4618,48 +4617,40 @@ def get_value(self, series, key): # would convert to numpy arrays and raise later any way) - GH29926 raise InvalidIndexError(key) - # if we have something that is Index-like, then - # use this, e.g. DatetimeIndex - # Things like `Series._get_value` (via .at) pass the EA directly here. - s = extract_array(series, extract_numpy=True) - if isinstance(s, ExtensionArray): + try: # GH 20882, 21257 # First try to convert the key to a location # If that fails, raise a KeyError if an integer # index, otherwise, see if key is an integer, and # try that - try: - iloc = self.get_loc(key) - return s[iloc] - except KeyError: - if len(self) > 0 and (self.holds_integer() or self.is_boolean()): - raise - elif is_integer(key): - return s[key] - - k = self._convert_scalar_indexer(key, kind="getitem") - try: - loc = self._engine.get_loc(k) - - except KeyError as e1: + loc = self._engine.get_loc(key) + except KeyError: if len(self) > 0 and (self.holds_integer() or self.is_boolean()): raise - - try: - return libindex.get_value_at(s, key) - except IndexError: + elif is_integer(key): + # If the Index cannot hold integer, then this is unambiguously + # a locational lookup. + loc = key + else: raise - except Exception: - raise e1 - except TypeError: - # e.g. "[False] is an invalid key" - raise IndexError(key) - else: - if is_scalar(loc): - tz = getattr(series.dtype, "tz", None) - return libindex.get_value_at(s, loc, tz=tz) - return series.iloc[loc] + return self._get_values_for_loc(series, loc) + + def _get_values_for_loc(self, series, loc): + """ + Do a positional lookup on the given Series, returning either a scalar + or a Series. + + Assumes that `series.index is self` + """ + if is_scalar(loc): + if isinstance(series._values, np.ndarray): + # Since we have an ndarray and not DatetimeArray, we dont + # have to worry about a tz. + return libindex.get_value_at(series._values, loc, tz=None) + return series._values[loc] + + return series.iloc[loc] def set_value(self, arr, key, value): """ diff --git a/pandas/core/indexes/datetimes.py b/pandas/core/indexes/datetimes.py index 91b9aa63c6a8e..53e3cc436d513 100644 --- a/pandas/core/indexes/datetimes.py +++ b/pandas/core/indexes/datetimes.py @@ -667,9 +667,8 @@ def get_value(self, series, key): return com.maybe_box(self, value, series, key) def get_value_maybe_box(self, series, key): - key = self._maybe_cast_for_get_loc(key) - values = self._engine.get_value(com.values_from_object(series), key, tz=self.tz) - return com.maybe_box(self, values, series, key) + loc = self.get_loc(key) + return self._get_values_for_loc(series, loc) def get_loc(self, key, method=None, tolerance=None): """ diff --git a/pandas/core/indexes/timedeltas.py b/pandas/core/indexes/timedeltas.py index e7427438828a8..45f98eaf34e40 100644 --- a/pandas/core/indexes/timedeltas.py +++ b/pandas/core/indexes/timedeltas.py @@ -259,8 +259,8 @@ def get_value(self, series, key): return com.maybe_box(self, value, series, key) def get_value_maybe_box(self, series, key: Timedelta): - values = self._engine.get_value(com.values_from_object(series), key) - return com.maybe_box(self, values, series, key) + loc = self.get_loc(key) + return self._get_values_for_loc(series, loc) def get_loc(self, key, method=None, tolerance=None): """ diff --git a/pandas/tests/indexes/datetimes/test_indexing.py b/pandas/tests/indexes/datetimes/test_indexing.py index 4c600e510790a..f3c255d50aba1 100644 --- a/pandas/tests/indexes/datetimes/test_indexing.py +++ b/pandas/tests/indexes/datetimes/test_indexing.py @@ -621,17 +621,21 @@ def test_get_value(self): # specifically make sure we have test for np.datetime64 key dti = pd.date_range("2016-01-01", periods=3) - arr = np.arange(6, 8) + arr = np.arange(6, 9) + ser = pd.Series(arr, index=dti) key = dti[1] - result = dti.get_value(arr, key) + with pytest.raises(AttributeError, match="has no attribute '_values'"): + dti.get_value(arr, key) + + result = dti.get_value(ser, key) assert result == 7 - result = dti.get_value(arr, key.to_pydatetime()) + result = dti.get_value(ser, key.to_pydatetime()) assert result == 7 - result = dti.get_value(arr, key.to_datetime64()) + result = dti.get_value(ser, key.to_datetime64()) assert result == 7 def test_get_loc(self): diff --git a/pandas/tests/indexes/test_base.py b/pandas/tests/indexes/test_base.py index 1047c457d6b82..d40a2257771a2 100644 --- a/pandas/tests/indexes/test_base.py +++ b/pandas/tests/indexes/test_base.py @@ -1915,7 +1915,12 @@ def test_get_value(self, index): values = np.random.randn(100) value = index[67] - tm.assert_almost_equal(index.get_value(values, value), values[67]) + with pytest.raises(AttributeError, match="has no attribute '_values'"): + # Index.get_value requires a Series, not an ndarray + index.get_value(values, value) + + result = index.get_value(Series(values, index=values), value) + tm.assert_almost_equal(result, values[67]) @pytest.mark.parametrize("values", [["foo", "bar", "quux"], {"foo", "bar", "quux"}]) @pytest.mark.parametrize( From 59b69769d9dd478f062ace42b9f6658b83d6d54b Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Sun, 19 Jan 2020 08:49:30 -0800 Subject: [PATCH 2/2] is_scalar -> is_integer --- pandas/core/indexes/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/indexes/base.py b/pandas/core/indexes/base.py index 030cda84dfebe..ae9aacb8f5301 100644 --- a/pandas/core/indexes/base.py +++ b/pandas/core/indexes/base.py @@ -4643,7 +4643,7 @@ def _get_values_for_loc(self, series, loc): Assumes that `series.index is self` """ - if is_scalar(loc): + if is_integer(loc): if isinstance(series._values, np.ndarray): # Since we have an ndarray and not DatetimeArray, we dont # have to worry about a tz.