BUG-20629 allow .at accessor with CategoricalIndex #26298

JustinZhengBC · 2019-05-06T20:43:42Z

closes Series.at and DataFrame.at crash with CategoricalIndex #20629
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
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.

codecov · 2019-05-06T21:22:25Z

Codecov Report

Merging #26298 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #26298      +/-   ##
==========================================
- Coverage   91.98%   91.98%   -0.01%     
==========================================
  Files         175      175              
  Lines       52374    52381       +7     
==========================================
+ Hits        48178    48181       +3     
- Misses       4196     4200       +4

Flag	Coverage Δ
#multiple	`90.53% <100%> (ø)`	⬆️
#single	`40.71% <0%> (-0.16%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/category.py	`98.62% <100%> (ø)`	⬆️
pandas/core/frame.py	`96.91% <100%> (-0.11%)`	⬇️
pandas/io/gbq.py	`78.94% <0%> (-10.53%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94c8c94...1843a04. Read the comment docs.

codecov · 2019-05-06T21:22:26Z

Codecov Report

Merging #26298 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #26298      +/-   ##
==========================================
- Coverage   91.74%   91.74%   -0.01%     
==========================================
  Files         174      174              
  Lines       50748    50755       +7     
==========================================
+ Hits        46560    46563       +3     
- Misses       4188     4192       +4

Flag	Coverage Δ
#multiple	`90.25% <100%> (ø)`	⬆️
#single	`41.69% <38.46%> (-0.09%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/category.py	`98.62% <100%> (ø)`	⬆️
pandas/core/frame.py	`97.02% <100%> (-0.11%)`	⬇️
pandas/_typing.py	`100% <100%> (ø)`	⬆️
pandas/io/gbq.py	`78.94% <0%> (-10.53%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b99e84f...7b598c6. Read the comment docs.

jreback · 2019-05-07T00:30:00Z

pandas/core/frame.py

+        except KeyError as e:
+            if self.index.nlevels > 1:
+                raise e  # partial indexing forbidden
+            else:


isn't this else surperflous?

jreback · 2019-05-07T00:32:01Z

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:


actually don't need the e here (just do raise) is enough

jreback · 2019-05-07T00:32:22Z

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


can you put the comment on a separate line before the raise

jreback · 2019-05-07T00:32:46Z

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:


hmm, what is the attribute error you are catching?

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.

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

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.

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.

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)

jreback · 2019-05-07T00:33:21Z

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):


test both loc and at with here

jreback · 2019-05-07T00:34:59Z

pandas/tests/indexing/test_categorical.py

+
+    def test_at_with_categorical_index(self):
+        # GH 20629
+        s = Series([1, 2, 3], index=pd.CategoricalIndex(["A", "B", "C"]))


move this after the loc tests

jreback · 2019-05-07T00:36:14Z

this might be solved as well: #14865

JustinZhengBC · 2019-05-07T06:01:04Z

This PR does not solve #14865 (although I'd be happy to look into it). There is still an issue with Series.get where if the key is not found in the index but can be interpreted to be a positional indexer, then it will be (for example, s.get(0) returns the first element when s has a CategoricalIndex that does not contain a 0) which is inconsistent which regular indexes.

JustinZhengBC · 2019-05-07T06:13:13Z

@jreback I have made the requested changes

…nto BUG-20629

jreback · 2019-05-16T01:42:34Z

pandas/core/indexes/category.py

@@ -488,16 +490,31 @@ def get_loc(self, key, method=None):
        except KeyError:
            raise KeyError(key)

-    def get_value(self, series, key):
+    def get_value(self,
+                  series: Union[ABCSeries, ExtensionArray, Index, np.ndarray],


can you add this type to pandas._typing, call it AnyArrayLike

jreback

minor typing comment, ping on green.

JustinZhengBC · 2019-05-16T06:23:25Z

@jreback green

jreback · 2019-05-19T19:25:08Z

lgtm. can you merge master and ping on green.

jreback · 2019-05-20T00:24:05Z

thanks @JustinZhengBC keep em coming!

BUG-20629 allow .at accessor with CategoricalIndex

1843a04

jreback requested changes May 7, 2019

View reviewed changes

jreback added Bug Categorical Categorical Data Type Indexing Related to indexing on series/frames, not to indexes themselves labels May 7, 2019

make requested changes

13717db

JustinZhengBC added 4 commits May 12, 2019 16:46

add type info, use take instead of iloc

188ac73

fix import order

41bf854

fix import order

cb0d4d9

Merge branch 'BUG-20629' of https://github.com/JustinZhengBC/pandas i…

3d38a5f

…nto BUG-20629

jreback added this to the 0.25.0 milestone May 16, 2019

jreback approved these changes May 16, 2019

View reviewed changes

jreback reviewed May 16, 2019

View reviewed changes

jreback approved these changes May 16, 2019

View reviewed changes

JustinZhengBC added 3 commits May 15, 2019 22:02

add AnyArrayLike to pandas._typing

52990a8

remove unnecessary type

14d016f

fix import order

2831d3d

Merge branch 'master' into BUG-20629

7b598c6

jreback merged commit 2b32e41 into pandas-dev:master May 20, 2019

jbrockmendel mentioned this pull request Jul 22, 2019

Partial string indexing returns ndarray rather than Series. #27516

Closed

topper-123 mentioned this pull request Nov 26, 2019

.at does not work using a categorical index. #25881

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG-20629 allow .at accessor with CategoricalIndex #26298

BUG-20629 allow .at accessor with CategoricalIndex #26298

JustinZhengBC commented May 6, 2019

codecov bot commented May 6, 2019

codecov bot commented May 6, 2019 •

edited

Loading

jreback May 7, 2019

jreback May 7, 2019

jreback May 7, 2019

jreback May 7, 2019

JustinZhengBC May 7, 2019

jreback May 7, 2019

JustinZhengBC May 7, 2019

jreback May 12, 2019

JustinZhengBC May 14, 2019

jreback May 7, 2019

jreback May 7, 2019

jreback commented May 7, 2019

JustinZhengBC commented May 7, 2019

JustinZhengBC commented May 7, 2019

jreback May 16, 2019

jreback left a comment

JustinZhengBC commented May 16, 2019

jreback commented May 19, 2019

jreback commented May 20, 2019

BUG-20629 allow .at accessor with CategoricalIndex #26298

BUG-20629 allow .at accessor with CategoricalIndex #26298

Conversation

JustinZhengBC commented May 6, 2019

codecov bot commented May 6, 2019

Codecov Report

codecov bot commented May 6, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented May 7, 2019

JustinZhengBC commented May 7, 2019

JustinZhengBC commented May 7, 2019

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

JustinZhengBC commented May 16, 2019

jreback commented May 19, 2019

jreback commented May 20, 2019

codecov bot commented May 6, 2019 •

edited

Loading