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

REF: collect methods in NumericIndex #41472

Merged
merged 4 commits into from
May 18, 2021

Conversation

topper-123
Copy link
Contributor

@topper-123 topper-123 commented May 14, 2021

Seperates the refactoring of existing methods out of #41153. Will make it easier to see what #41153 actually brings in of new things.

This PR changes no functionality, but only makes the exisiting numeric indexes into thin subclasses of the existing NumericIndex.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

looks good a couple of comments

else:
return False

@property
Copy link
Contributor

Choose a reason for hiding this comment

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

cached_property?

np.float64: libindex.Float64Engine,
}[self.dtype.type]

@property
Copy link
Contributor

Choose a reason for hiding this comment

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

cached_property

pandas/core/indexes/numeric.py Show resolved Hide resolved
@@ -255,6 +357,14 @@ def asi8(self) -> np.ndarray:
return self._values.view(self._default_dtype)


_int64_descr_args = {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you put these in the class?

@jreback jreback added Index Related to the Index class or subclasses Refactor Internal refactoring of code labels May 17, 2021
@topper-123
Copy link
Contributor Author

Updated.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm. @jbrockmendel if any comments

@jreback jreback added this to the 1.3 milestone May 17, 2021
@@ -150,13 +296,16 @@ def _is_comparable_dtype(self, dtype: DtypeObj) -> bool:
return is_numeric_dtype(dtype)

@classmethod
def _assert_safe_casting(cls, data, subarr):
def _assert_safe_casting(cls, data, subarr) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

subarr is ndarray?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and data too, I can add those.

@jbrockmendel
Copy link
Member

LGTM; if you find anywhere where @final can be correctly added, pls do

@topper-123
Copy link
Contributor Author

Wrt. @Final, I’m not sure I like it in library code.

Couldn’t a third party subclass an index and change one of methods in an unanticipated way, but the @Final be in the way of that?

@topper-123
Copy link
Contributor Author

Updated type hints.

@jreback jreback merged commit 09f3bf8 into pandas-dev:master May 18, 2021
@jreback
Copy link
Contributor

jreback commented May 18, 2021

thanks @topper-123

@topper-123 topper-123 deleted the ref_numeric_index branch May 18, 2021 13:56
topper-123 added a commit to topper-123/pandas that referenced this pull request May 20, 2021
TLouf pushed a commit to TLouf/pandas that referenced this pull request Jun 1, 2021
topper-123 added a commit to topper-123/pandas that referenced this pull request Jun 3, 2021
topper-123 added a commit to topper-123/pandas that referenced this pull request Jun 4, 2021
topper-123 added a commit to topper-123/pandas that referenced this pull request Jun 9, 2021
topper-123 added a commit to topper-123/pandas that referenced this pull request Jun 21, 2021
topper-123 added a commit that referenced this pull request Jul 3, 2021
JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
topper-123 added a commit to topper-123/pandas that referenced this pull request Jul 14, 2021
topper-123 added a commit to topper-123/pandas that referenced this pull request Jul 24, 2021
topper-123 added a commit to topper-123/pandas that referenced this pull request Jul 25, 2021
topper-123 added a commit to topper-123/pandas that referenced this pull request Jul 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Index Related to the Index class or subclasses Refactor Internal refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants