-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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 conftest & _testing #49678
DEPR: remove Int|Uint|Float64Index from conftest & _testing #49678
Conversation
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.
looks good, just got a question (more for my understanding than anything else)
"int64", | ||
"int32", |
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 for my understanding, why don't 'int16'
and 'int8'
need to be here as well? (likewise for other tests)
this isn't a request to change it, just trying to understand
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 thought it was a bit strange to test these things here, and suspect many of the tests here should be located in pandas.tests.indexes.common.Base
and adapted in its subclasses as needed. At the same time I didn't really want to delve too deep into refactoring the code base, as it would distract from my main objective right now i.e. getting rid of the old numeric indexes. I probably shouldn't even have added the 32bit versions, but I was curious if it would still pass or some issue would show up.
How about I write an issue about this refactoring and fix it after I've removed Int64Index & friends?
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.
yeah no objections
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.
Thanks, I've written a issue, see #49679.
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.
As far I can tell this is fine
Thanks @topper-123 |
…ev#49678) * DEPR: remove Int|Uint|Float64Index from conftest + related * fix failures * fix flake8 Co-authored-by: Terji Petersen <terjipetersen@Terjis-Air.fritz.box>
…ev#49678) * DEPR: remove Int|Uint|Float64Index from conftest + related * fix failures * fix flake8 Co-authored-by: Terji Petersen <terjipetersen@Terjis-Air.fritz.box>
Progress towards #42717.
In particular cleans up conftest.py::indices_dict to not use old style indexes and cleans up the
tm.make*Index
functions to not return old style numeric indexes.I've kept
tm.MakeIntIndex
etc. and just made them tighter versions oftm.makeNumericIndex
though if they should be deleted that is also fine for me.