-
-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
Simplify to_pydatetime() #17592
Simplify to_pydatetime() #17592
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17592 +/- ##
==========================================
- Coverage 91.19% 91.18% -0.02%
==========================================
Files 163 163
Lines 49627 49627
==========================================
- Hits 45259 45250 -9
- Misses 4368 4377 +9
Continue to review full report at Codecov.
|
pandas/_libs/tslib.pyx
Outdated
dts.us, ts.tzinfo) | ||
|
||
return datetime(self.year, self.month, self.day, | ||
self.hour, self.minute, self.second, |
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.
indendation
LGTM, minus formatting nit. |
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.
can you verify that there is an asv for this (and add one if not).
There is not, just added one. |
Hello @jbrockmendel! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on September 21, 2017 at 22:48 Hours UTC |
Looks like someone added one overnight, will add to_pydatetime timing to it. |
Is there an error message in the circleci log that I'm missing? |
looks fine you can click thru to circleci to see errors |
I don't ask just because I want to waste your time. I'm saying I don't see any error message in the log. |
I know, but some people don't actually realize you can click thru. yeah it prob timed out or something. Wait till I merge #17619 and rebase |
thanks! |
Up until #17331,
Timestamp.microsecond
was very slow. So previously it was faster to go throughconvert_to_tsobject
to get a newdatetime
instance than it was to just returndatetime(self.year, self.month, self.day, self.hour, self.minute, self.second, self.microsecond, self.tzinfo)
Now it is slightly faster (and much simpler) to do it this way.
Before:
After:
With a tz-aware Timestamp the speedup is much bigger:
Before:
After: