-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
PERF: RangeIndex.round returns RangeIndex when possible #57824
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
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 to me, added few suggestions.
pandas/core/indexes/range.py
Outdated
if decimals >= 0: | ||
return self.copy() | ||
elif all( | ||
getattr(self, attr) % 10**-decimals == 0 for attr in ("start", "step") |
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.
Personal preference, but I'd find more readable to simply use self.start % 10 ... and self.stop % 10 ...
.
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.
Fair point. I'll change this to your suggestion
pandas/core/indexes/range.py
Outdated
elif all( | ||
getattr(self, attr) % 10**-decimals == 0 for attr in ("start", "step") | ||
): | ||
# e.g. Range(10, 30, 10).round(-1) doesn't need rounding |
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.
# e.g. Range(10, 30, 10).round(-1) doesn't need rounding | |
# e.g. RangeIndex(10, 30, 10).round(-1) doesn't need rounding |
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!
pandas/core/indexes/range.py
Outdated
---------- | ||
decimals : int, optional | ||
Number of decimal places to round to. If decimals is negative, | ||
it specifies the number of positions to the left of the decimal point. |
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 know this is from the original docstring, but for me it's not obvious what this means by reading it, maybe adding an example or two here would be helpful.
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.
Sure thing. I added an example for the negative case.
…7824) * PERF: RangeIndex.round returns RangeIndex when possible * Add whatsnew * Add typing * Address feedback
No description provided.