-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
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
_TSObject
logic belongs intslibs.conversion
. To start off small, this just moves a few out-of-place functions from_libs.lib
and_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_timestamp
andtime64_to_datetime
:for i from 0 <= i < n:
becamefor i in range(n):
Right now the functions moved from
lib
are imported with a# noqa
. If OK, I'll follow-up by updating the imports elsewhere.git diff upstream/master -u -- "*.py" | flake8 --diff