Skip to content
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 NumericIndex #51139

Merged
merged 4 commits into from Feb 3, 2023
Merged

Conversation

topper-123
Copy link
Contributor

@topper-123 topper-123 commented Feb 3, 2023

This finishes the removal of NumericIndex from the code base. Note that this PR builds atop of #51132, so you might want to merge that first.

Everything should be finished now after this PR, e.g. the doc updates have been merged (#51111) and the doc string has been updated (#51089). I'll have a final look once this is merged, but this should pretty much finish this issue.

@topper-123 topper-123 changed the title DEPR: Remove numeric index DEPR: Remove NumericIndex Feb 3, 2023
@@ -5375,6 +5362,7 @@ def identical(self, other) -> bool:
for c in self._comparables
)
and type(self) == type(other)
and self.dtype == other.dtype
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously the index type could differentiate e.g. Index([1, 2, 3], dtype=object) and Index([1, 2, 3], dtype="int64") (the latter actually being a NumericIndex), but we now need to be more precise.

@@ -539,7 +538,6 @@ def __new__(

klass = cls._dtype_to_subclass(arr.dtype)

# _ensure_array _may_ be unnecessary once NumericIndex etc are gone
arr = klass._ensure_array(arr, arr.dtype, copy=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not unnecessary?

Copy link
Contributor Author

@topper-123 topper-123 Feb 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to remove “NumericIndex” and had to decide what to do with comment and opted to delete it. IMO _ensure_array could be refactored, but so could a lot of other things in pandas and _ensure_array is not that bad anymore compared to other things, so doesn’t merit a comment In the code base IMO.

This was my reasoning, others could of course have made a different judgement call…So it’s not a strong opinion, and I’m fine with it being refactored, but also it not being refactored :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah no worries, just wasn't sure if this was still being True or not

Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small comment (can also be addressed in a follow up), otherwise lgtm

@phofl phofl added the Deprecate Functionality to remove in pandas label Feb 3, 2023
@phofl phofl added this to the 2.0 milestone Feb 3, 2023
@phofl phofl merged commit 8cb6382 into pandas-dev:main Feb 3, 2023
@phofl
Copy link
Member

phofl commented Feb 3, 2023

thx @topper-123

@mwtoews
Copy link
Contributor

mwtoews commented May 19, 2023

This removal is the partially the cause of a regression unpickling files from "pandas<2", #53300

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas Index Related to the Index class or subclasses
Projects
None yet
3 participants