BUG: Check types in Index.__contains__ (#22085) #22602
Conversation
|
Hello @yeojin-dev! Thanks for updating the PR.
Comment last updated on September 19, 2018 at 12:28 Hours UTC |
|
If you fetch and merge master the CI failures should be fixed. |
jreback
added
Bug
Indexing
pandas/_libs/index.pyx
Outdated
| @@ -77,6 +77,9 @@ cdef class IndexEngine: | |||
| def __contains__(self, object val): | |||
| self._ensure_mapping_populated() | |||
| hash(val) | |||
| if (util.is_float_object(val) and isinstance(self, Int64Engine) and | |||
There was a problem hiding this comment.
can you just do this in the index_class_helper.pyi (just override the method there). Is this applicable for the UInt64Engine as well?
pandas/tests/indexes/test_base.py
Outdated
| @@ -2474,6 +2474,19 @@ 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): | |||
There was a problem hiding this comment.
move to pandas/tests/indexes/test_indexing.py (near the other contains tests). Use an assert on the constructed Index itself (e.g. assert its an Int64Index or FloatIndex), or you can just construct them directly. Test on UInt64 as well.
There was a problem hiding this comment.
name these a bit more intuitively, e.g. integer_index, float_index
Codecov Report
@@ Coverage Diff @@
## master #22602 +/- ##
==========================================
+ Coverage 92.16% 92.16% +<.01%
==========================================
Files 169 169
Lines 50769 50778 +9
==========================================
+ Hits 46791 46800 +9
Misses 3978 3978
Continue to review full report at Codecov.
|
|
@jreback Please check. 😆 |
| @@ -80,4 +80,13 @@ cdef class {{name}}Engine(IndexEngine): | |||
|
|
|||
| {{endif}} | |||
|
|
|||
| {{if name == 'Int64' or name == 'UInt64'}} | |||
| def __contains__(self, object val): | |||
There was a problem hiding this comment.
this is nice, can you add an else for all other dtypes.
Then I think can you edit pandas/core/indexes/base.py (and any sub-classes) that have a __contains__
and remove the hash check, and I think move the try/except to here.
There was a problem hiding this comment.
actually on 2nd thought, this might be better in pandas.core.indexes.numeric.py can you try there? (ignore my other comment)
There was a problem hiding this comment.
@jreback I've no idea what I should edit in numeric.py. There's no __contains__ except Float64Index. Did you mean what I should is to move my code to numeric.py from pxi.in?
Pipfile
Outdated
| @@ -0,0 +1,11 @@ | |||
| [[source]] | |||
| url = "https://pypi.org/simple" | |||
There was a problem hiding this comment.
Sorry, it's my mistake.
pandas/core/indexes/numeric.py
Outdated
| @@ -174,6 +175,12 @@ class Int64Index(NumericIndex): | |||
| _engine_type = libindex.Int64Engine | |||
| _default_dtype = np.int64 | |||
|
|
|||
| def __contains__(self, key): | |||
| hash(key) | |||
There was a problem hiding this comment.
can you create a IntegerIndex(NumericIndex) and put this method there, then inherit Int64Index and UInt64Index from that?
|
thanks @yeojin-dev sorry for the back and forth, sometimes hard to figure out where to put things |
|
@jreback Thanks. 😆 |
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.
And this is the same as #22360. I deleted the branch while rebasing it... 😅