Repository: pandas-dev/pandas
Pull request: BUG-20629 allow .at accessor with CategoricalIndex
- [X] closes #20629
- [X] tests added / passed
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

This PR alters exception handling in various `get_value` functions to make calls to `.at` on non-integer `CategoricalIndex` fall back on positional indexing methods instead of raising exceptions, allowing queries like those mentioned in #20629.

Problem statement:
Series.at and DataFrame.at crash with CategoricalIndex
#### Code Sample, a copy-pastable example if possible

```python
Python 3.6.3 (default, Oct  3 2017, 21:45:48) 
[GCC 7.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pandas as pd
>>> x = pd.Series([1, 2, 3], index=pd.CategoricalIndex(["A", "B", "C"]))
>>> x.loc["A"]
1
>>> x.at["A"]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.6/dist-packages/pandas/core/indexing.py", line 1869, in __getitem__
    return self.obj._get_value(*key, takeable=self._takeable)
  File "/usr/local/lib/python3.6/dist-packages/pandas/core/series.py", line 929, in _get_value
    return self.index.get_value(self._values, label)
  File "/usr/local/lib/python3.6/dist-packages/pandas/core/indexes/category.py", line 423, in get_value
    return series.iloc[indexer]
AttributeError: 'numpy.ndarray' object has no attribute 'iloc'
>>> x = pd.DataFrame([[1, 2], [3, 4], [5, 6]], index=pd.CategoricalIndex(["A", "B", "C"]))
>>> x.loc["B", 1]
4
>>> x.at["B", 1]
Traceback (most recent call last):
  File "pandas/_libs/index.pyx", line 139, in pandas._libs.index.IndexEngine.get_loc
  File "pandas/_libs/hashtable_class_helper.pxi", line 811, in pandas._libs.hashtable.Int64HashTable.get_item
TypeError: an integer is required

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.6/dist-packages/pandas/core/indexing.py", line 1869, in __getitem__
    return self.obj._get_value(*key, takeable=self._takeable)
  File "/usr/local/lib/python3.6/dist-packages/pandas/core/frame.py", line 1985, in _get_value
    return engine.get_value(series._values, index)
  File "pandas/_libs/index.pyx", line 83, in pandas._libs.index.IndexEngine.get_value
  File "pandas/_libs/index.pyx", line 91, in pandas._libs.index.IndexEngine.get_value
  File "pandas/_libs/index.pyx", line 141, in pandas._libs.index.IndexEngine.get_loc
KeyError: 'B'
```

#### Problem description

With a `CategoricalIndex`, `Series.at` raises `AttributeError` and `DataFrame.at` raises `TypeError` and `KeyError`.

#### Expected Output

`x.at["A"]` should work the same as `x.loc["A"]`, and `x.at["B", 1]` should work the same as `x.loc["B", 1]`.

#### Output of ``pd.show_versions()``

<details>

commit: None
python: 3.6.3.final.0
python-bits: 64
OS: Linux
OS-release: 4.13.0-37-generic
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8

pandas: 0.22.0
pytest: 3.4.0
pip: 9.0.3
setuptools: 39.0.1
Cython: None
numpy: 1.14.2
scipy: 1.0.0
pyarrow: None
xarray: None
IPython: None
sphinx: 1.6.7
patsy: 0.4.1
dateutil: 2.7.2
pytz: 2018.3
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: 2.2.2
openpyxl: 2.4.9
xlrd: 1.1.0
xlwt: None
xlsxwriter: None
lxml: 4.0.0
bs4: 4.6.0
html5lib: 1.0.1
sqlalchemy: 1.2.0
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None

</details>

Hints from issue discussion:
Thanks for the report, that's clearly a bug. 

Related issue (on indexing with categorical index): https://github.com/pandas-dev/pandas/issues/14865

Commit message: BUG-20629 allow .at accessor with CategoricalIndex

Patch to review:
diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst
index a65b92737c220..1a0fca3fa0ec2 100644
--- a/doc/source/whatsnew/v0.25.0.rst
+++ b/doc/source/whatsnew/v0.25.0.rst
@@ -262,7 +262,7 @@ Bug Fixes
 Categorical
 ^^^^^^^^^^^
 
--
+- Bug in :func:`DataFrame.at` and :func:`Series.at` that would raise exception if the index was a :class:`CategoricalIndex` (:issue:`20629`)
 -
 -
 
diff --git a/pandas/core/frame.py b/pandas/core/frame.py
index 09f5726e6341f..856614b9bc2f3 100644
--- a/pandas/core/frame.py
+++ b/pandas/core/frame.py
@@ -2710,13 +2710,19 @@ def _get_value(self, index, col, takeable=False):
 
         try:
             return engine.get_value(series._values, index)
+        except KeyError as e:
+            if self.index.nlevels > 1:
+                raise e  # partial indexing forbidden
+            else:
+                pass  # GH 20629
         except (TypeError, ValueError):
+            pass
 
-            # we cannot handle direct indexing
-            # use positional
-            col = self.columns.get_loc(col)
-            index = self.index.get_loc(index)
-            return self._get_value(index, col, takeable=True)
+        # we cannot handle direct indexing
+        # use positional
+        col = self.columns.get_loc(col)
+        index = self.index.get_loc(index)
+        return self._get_value(index, col, takeable=True)
     _get_value.__doc__ = get_value.__doc__
 
     def set_value(self, index, col, value, takeable=False):
diff --git a/pandas/core/indexes/category.py b/pandas/core/indexes/category.py
index 473686a7541a5..0405adebf02c0 100644
--- a/pandas/core/indexes/category.py
+++ b/pandas/core/indexes/category.py
@@ -498,6 +498,8 @@ def get_value(self, series, key):
             k = self._convert_scalar_indexer(k, kind='getitem')
             indexer = self.get_loc(k)
             return series.iloc[indexer]
+        except AttributeError:
+            return super().get_value(series, indexer)
         except (KeyError, TypeError):
             pass
 
diff --git a/pandas/tests/indexing/test_categorical.py b/pandas/tests/indexing/test_categorical.py
index 36a5f803237a2..fda850222833b 100644
--- a/pandas/tests/indexing/test_categorical.py
+++ b/pandas/tests/indexing/test_categorical.py
@@ -719,3 +719,11 @@ def test_map_with_dict_or_series(self):
         output = cur_index.map(mapper)
         # Order of categories in output can be different
         tm.assert_index_equal(expected, output)
+
+    def test_at_with_categorical_index(self):
+        # GH 20629
+        s = Series([1, 2, 3], index=pd.CategoricalIndex(["A", "B", "C"]))
+        assert s.at['A'] == 1
+        df = DataFrame([[1, 2], [3, 4], [5, 6]],
+                       index=pd.CategoricalIndex(["A", "B", "C"]))
+        assert df.at['B', 1] == 4


Code review comments:
diff_hunk: @@ -2710,13 +2710,19 @@ def _get_value(self, index, col, takeable=False):
 
         try:
             return engine.get_value(series._values, index)
+        except KeyError as e:
+            if self.index.nlevels > 1:
+                raise e  # partial indexing forbidden
+            else:
path: pandas/core/frame.py
text: @user1:
isn't this else surperflous?

diff_hunk: @@ -719,3 +719,11 @@ def test_map_with_dict_or_series(self):
         output = cur_index.map(mapper)
         # Order of categories in output can be different
         tm.assert_index_equal(expected, output)
+
+    def test_at_with_categorical_index(self):
+        # GH 20629
+        s = Series([1, 2, 3], index=pd.CategoricalIndex(["A", "B", "C"]))
path: pandas/tests/indexing/test_categorical.py
text: @user1:
move this after the loc tests

diff_hunk: @@ -719,3 +719,11 @@ def test_map_with_dict_or_series(self):
         output = cur_index.map(mapper)
         # Order of categories in output can be different
         tm.assert_index_equal(expected, output)
+
+    def test_at_with_categorical_index(self):
path: pandas/tests/indexing/test_categorical.py
text: @user1:
test both loc and at with here

diff_hunk: @@ -498,6 +498,8 @@ def get_value(self, series, key):
             k = self._convert_scalar_indexer(k, kind='getitem')
             indexer = self.get_loc(k)
             return series.iloc[indexer]
+        except AttributeError:
path: pandas/core/indexes/category.py
text: @user1:
hmm, what is the attribute error you are catching?

@author:
The above line, `return series.iloc[indexer]`, assumes the `series` argument has implemented `iloc`. The regular index version does not make this assumption. In addition, the doc for this function reads "Fast lookup of value from 1-dimensional ndarray" so this assumption should be relaxed.

@user1:
ok,, I would then put the AttributeError catching with the Key and Type error below then.?

@author:
The key and type errors catch exceptions from `self.get_loc`, which usually happens when using indexing like `s[s>5]` leading to a key which is like a list of booleans.

The exceptions cannot use `super.get_value(series, key)` because its implementation does not work for `CategoricalIndex` when the key is a scalar value in the index (although it does work when the key is a positional index).

They also cannot all use `super.get_value(series, indexer)` because the creation of `indexer` is contingent on `self.get_loc` not failing.

Therefore, two cases are needed.

@user1:
> The above line, return series.iloc[indexer], assumes the series argument has implemented iloc. The regular index version does not make this assumption. 

ok so IIUC, ``series`` can actually be an ``Index`` or a ``Series``? if so, then just change to using ``.take()`` I think should work. Can you try and type annotate things (and update the doc-strings). 

I don't think we would need the AttributeError catching then.

@author:
You're right. I've applied your recommended changes and added type information (the case where `series` is an `ndarray` was actually the case in the original issue)

diff_hunk: @@ -2710,13 +2710,19 @@ def _get_value(self, index, col, takeable=False):
 
         try:
             return engine.get_value(series._values, index)
+        except KeyError as e:
path: pandas/core/frame.py
text: @user1:
actually don't need the e here (just do ``raise``) is enough