-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ENH: non-nano Timestamp.timestamp, to_period #46990
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
@@ -348,6 +348,19 @@ cdef int64_t periods_per_day(NPY_DATETIMEUNIT reso=NPY_DATETIMEUNIT.NPY_FR_ns) e | |||
return day_units | |||
|
|||
|
|||
cdef int64_t periods_per_second(NPY_DATETIMEUNIT reso) except? -1: |
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 can't be defined with periods_per_day
because of timezones?
@@ -348,6 +348,19 @@ cdef int64_t periods_per_day(NPY_DATETIMEUNIT reso=NPY_DATETIMEUNIT.NPY_FR_ns) e | |||
return day_units | |||
|
|||
|
|||
cdef int64_t periods_per_second(NPY_DATETIMEUNIT reso) except? -1: |
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 can't be defined with periods_per_day
because of timezones?
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.
we could share for some cases, but for not for e.g. NPY_FR_h
|
||
if self._reso != NPY_FR_ns: | ||
raise NotImplementedError(self._reso) | ||
|
||
normalized = normalize_i8_stamp(local_val) | ||
normalized = normalize_i8_stamp(local_val, ppd) |
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.
Do we have non-nano tests for normalize()
?
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.
not yet, still raises a few lines up, per commit message this is preliminary
|
||
if self._reso != NPY_FR_ns: | ||
raise NotImplementedError(self._reso) | ||
|
||
normalized = normalize_i8_stamp(local_val) | ||
normalized = normalize_i8_stamp(local_val, ppd) |
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.
Do we have non-nano tests for normalize()
?
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.
Great sounds good.
* ENH: Timestamp.timestamp non-nano * ENH: Timestamp.to_period non-nano * normalize prelims
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.