-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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
DEPR: remove Int/Uint/Float64Index from pandas/tests/indexes/ranges #50826
DEPR: remove Int/Uint/Float64Index from pandas/tests/indexes/ranges #50826
Conversation
pandas/core/indexes/range.py
Outdated
@@ -48,7 +48,6 @@ | |||
import pandas.core.indexes.base as ibase | |||
from pandas.core.indexes.base import maybe_extract_name | |||
from pandas.core.indexes.numeric import ( | |||
Float64Index, | |||
Int64Index, |
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.
Is this intended?
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.
No, I somehow missed this. I'll update.
@@ -30,9 +29,9 @@ def test_join_outer(self): | |||
dtype=np.intp, | |||
) | |||
|
|||
assert isinstance(res, Int64Index) | |||
assert isinstance(res, Index) and res.dtype == np.int64 |
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 have a function to check for int64, any_int64_dtype I think
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.
I found is_int64_dtype
. IMO using that function is just a less clear way to check the same thing as I already do. Is there a benefit to using that function over checking directly?
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.
consistency and this would also work for Int64 for example
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.
Ok, I do think the other is better because we test specifically for np.int64
here, and pd.Int64
should give an error in this case. But not a big issue for me, I'ver updated.
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.
small comments
be5519f
to
a88882e
Compare
expected = Int64Index([1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 12, 13, 14]) | ||
tm.assert_index_equal(result, expected) | ||
expected = Index([1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 12, 13, 14]) | ||
tm.assert_index_equal(result, expected, exact=True) |
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.
Just confirming that exact=True
still checks the exact Index class as well. (The docs still mention Int64Index but not sure if any changes were needed)
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.
tm.assert_index_equal
works as expected, e.g.
>>> import pandas as pd
>>> import pandas._testing as tm
>>> idx = pd.Index([*range(1, 15)])
>>> ri = pd.RangeIndex(1, 15)
>>> tm.assert_index_equal(idx, ri, exact=True)
AssertionError: Index are different
Index classes are different
[left]: NumericIndex([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14], dtype='int64')
[right]: RangeIndex(start=1, stop=15, step=1)
The _testing
module has to be cleaned up also, I'll do that soon.
Thanks @topper-123 |
…andas-dev#50826) * remove Int/Uint/Float64Index from pandas/tests/indexes/ranges * remove Int64Index * is_int64_dtype * pre-commit
Extracted from #50479 to make it more manageable.
Progress towards #42717.