-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
Move time conversion funcs to tslibs.conversion #17708
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
jreback
left a comment
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 think you can remove all of these with 1 change as noted below
| return gmtime(dt) | ||
|
|
||
|
|
||
| def array_to_timestamp(ndarray[object, ndim=1] arr): |
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 don't think this is called anywhere?
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.
Doesn't look like it, no. But its def and not cdef, so conceivably used downstream. OK to remove?
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.
yes ok to remove
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.
these are ALL private modules (did that on purpose), so can move code around as long as we are internally consistent
| return result | ||
|
|
||
|
|
||
| def time64_to_datetime(ndarray[int64_t, ndim=1] arr): |
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.
this is just a simple call to pd.to_datetime
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.
This is only used in io.pytables. OK to replace then remove?
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.
yes, I think you can just replace that call in pytables
|
Looks like |
|
and |
|
@jbrockmendel yep happy to delete code |
Eventually a bunch of
_TSObjectlogic belongs intslibs.conversion. To start off small, this just moves a few out-of-place functions from_libs.liband_libs.index.The changes that are not cut/paste:
In
_to_i8:tslib._delta_to_nanoseconds(offset)becameint(offset.total_seconds() * 1e9)In
array_to_timestampandtime64_to_datetime:for i from 0 <= i < n:becamefor i in range(n):Right now the functions moved from
libare imported with a# noqa. If OK, I'll follow-up by updating the imports elsewhere.git diff upstream/master -u -- "*.py" | flake8 --diff