-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: loc-indexing with a CategoricalIndex with non-string categories #29922
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
BUG: loc-indexing with a CategoricalIndex with non-string categories #29922
Conversation
49eb424
to
5167024
Compare
Hello @topper-123! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-12-11 07:13:21 UTC |
5167024
to
364f61e
Compare
pandas/core/indexes/category.py
Outdated
try: | ||
return self.categories._convert_scalar_indexer(key, kind=kind) | ||
except TypeError: | ||
self._invalid_indexer("label", key=key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when is this TypeError hit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was hit when you have pd.Series([1,2], index=pd.CategoricalIndex(["a", "b"])).loc[1]
, i.e. wrong indexing type. It's the same error type as in s = pd.Series([1,2], index=["a", "b"]).loc[1]
.
I've added a test for it.
4751ca7
to
f2274d4
Compare
pandas/core/indexes/category.py
Outdated
if self.categories._defer_to_indexing: | ||
return self.categories._convert_scalar_indexer(key, kind=kind) | ||
|
||
if kind == "loc" or self.categories._defer_to_indexing: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we might not need the _defer_to_indexing any longer, as I think the only way to get here is kind=='loc'
Ill be abroad untill the weekend, will look at it then. I think it would cause a error with categories made from datetimeindex, but I’ll check. |
e9f5832
to
746d6d4
Compare
I`ve updated the PR: I´ve removed _defer_to_indexing and added some tests. |
result = df.loc[idx_values[0]] | ||
expected = Series(["foo"], index=["A"], name=idx_values[0]) | ||
tm.assert_series_equal(result, expected) | ||
# list selection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you put a blank line between cases
np.array([1, 2, 3], dtype=dtype) | ||
for dtype in [ | ||
np.int8, | ||
np.int16, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any way to use the fixtures (or definitions) in pandas/conftest.py for some of this
746d6d4
to
7316414
Compare
I've updated and use the dtypes from pandas/conftest.py instead. |
[1.5, 2.5, 3.5], | ||
[-1.5, -2.5, -3.5], | ||
# numpy int/uint | ||
*[np.array([1, 2, 3], dtype=dtype) for dtype in conftest.ALL_INT_DTYPES], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great! nice comprehensive test
thanks @topper-123 very nice! |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
Bug when indexing with
.loc
and the index is a CategoricalIndex with integer or float categories.