BUG: Check types in Index.__contains__ (#22085) #22360
Conversation
|
Hello @yeojin-dev! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on August 30, 2018 at 15:29 Hours UTC |
Codecov Report
@@ Coverage Diff @@
## master #22360 +/- ##
=========================================
Coverage ? 92.04%
=========================================
Files ? 169
Lines ? 50783
Branches ? 0
=========================================
Hits ? 46741
Misses ? 4042
Partials ? 0
Continue to review full report at Codecov.
|
|
note that this duplicates #22366 need tests & a whatsnew entry |
jreback
added
Bug
Indexing
|
@jreback Thank you for checking my PR. I added test and whatsnew. I hope these may help you. 😊 |
doc/source/whatsnew/v0.24.0.txt
Outdated
| @@ -653,6 +653,7 @@ Indexing | |||
| - Fixed ``DataFrame[np.nan]`` when columns are non-unique (:issue:`21428`) | |||
| - Bug when indexing :class:`DatetimeIndex` with nanosecond resolution dates and timezones (:issue:`11679`) | |||
| - Bug where indexing with a Numpy array containing negative values would mutate the indexer (:issue:`21867`) | |||
| - Bug in :func:`Index.__contains__` that returns True just in case val is float and dtype is integer, because float casted to integer (:issue:`22085`) | |||
There was a problem hiding this comment.
something more along the line of
Bug in 'scalar in Index' if scalar is a float while the Index is of integer dtype.
pandas/core/indexes/base.py
Outdated
| @@ -1949,7 +1949,11 @@ def __nonzero__(self): | |||
| def __contains__(self, key): | |||
| hash(key) | |||
| try: | |||
| return key in self._engine | |||
| if is_float(key) and is_integer_dtype(self.dtype) and\ | |||
There was a problem hiding this comment.
don't use \ for line continuation, instead use parens
pandas/tests/indexes/test_base.py
Outdated
| @@ -2483,6 +2483,14 @@ def test_comparison_tzawareness_compat(self, op): | |||
| # TODO: implement _assert_tzawareness_compat for the reverse | |||
| # comparison with the Series on the left-hand side | |||
|
|
|||
| def test_contains_with_float_val(self): | |||
| # GH#22085 | |||
| index = pd.Index([0, 1, 2, 3]) | |||
There was a problem hiding this comment.
can you also add a Float index and test the same cases
|
@jreback I fixed them. Please check. 😊 |
|
@jreback Could you please give me your feedback on my last commit? |
doc/source/whatsnew/v0.24.0.txt
Outdated
| @@ -658,6 +658,7 @@ Indexing | |||
| - Bug when indexing :class:`DatetimeIndex` with nanosecond resolution dates and timezones (:issue:`11679`) | |||
| - Bug where indexing with a Numpy array containing negative values would mutate the indexer (:issue:`21867`) | |||
| - ``Float64Index.get_loc`` now raises ``KeyError`` when boolean key passed. (:issue:`19087`) | |||
| - Bug in `scalar in Index` if scalar is a float while the Index is of integer dtype (:issue:`22085`) | |||
There was a problem hiding this comment.
use double-backticks on the Index (2nd use in sentence)
pandas/core/indexes/base.py
Outdated
| if (is_float(key) and is_integer_dtype(self.dtype) and | ||
| int(key) != key): | ||
| return False | ||
| else: |
pandas/core/indexes/base.py
Outdated
| int(key) != key): | ||
| return False | ||
| else: | ||
| return key in self._engine |
There was a problem hiding this comment.
this is ok in python code, but actually can you see if you can do this in the cython code?
There was a problem hiding this comment.
Thanks. But I'm not good at Cython now. I'd like to try it but it'll take time.
|
@jreback My new commits has just passed a test in my Mac. But can't here, GitHub. See the logs below.
I guess these errors are not caused by my commits. Please check. |
|
@jreback I wrote the code in Cython. But my code has just passed the test, can't pass here. I can't understand... 😢 |
|
can you rebase |
|
@jreback Sorry. I deleted the branch while rebasing it. Please see my new PR. |
git diff upstream/master -u -- "*.py" | flake8 --diffI added is_float, is_integer_dtype in Index.__contains__.
If key is float and dtype of Index object is integer, return False.