BUG: Period[us] start_time off by 1 nanosecond #31475
Conversation
|
Updated to catch overflow correctly. |
| @@ -656,6 +656,23 @@ def test_conv_secondly(self): | |||
|
|
|||
| assert ival_S.asfreq("S") == ival_S | |||
|
|
|||
| def test_conv_microsecond(self): | |||
| # Avoid floating point errors dropping the start_time to before | |||
| # the beginning of the Period | |||
There was a problem hiding this comment.
Changes look good, whatsnew entry notwithstanding.
Reference the PR number (since there is no issue number) here.
| @@ -1186,6 +1186,13 @@ cdef int64_t period_ordinal_to_dt64(int64_t ordinal, int freq) except? -1: | |||
| if ordinal == NPY_NAT: | |||
| return NPY_NAT | |||
|
|
|||
| if freq == 11000: | |||
| # Microsecond, avoid get_date_info to prevent floating point errors | |||
There was a problem hiding this comment.
Is it possible to fix get_date_info instead?
There was a problem hiding this comment.
Maybe. My first two tries didnt work.
There was a problem hiding this comment.
so, is it possible to use this path for all freqs?
There was a problem hiding this comment.
definitely not for the lower-frequency freqs because of their to_end attrs e.g. Q-DEC, Q-NOV.
| @@ -1186,6 +1186,13 @@ cdef int64_t period_ordinal_to_dt64(int64_t ordinal, int freq) except? -1: | |||
| if ordinal == NPY_NAT: | |||
| return NPY_NAT | |||
|
|
|||
| if freq == 11000: | |||
| # Microsecond, avoid get_date_info to prevent floating point errors | |||
There was a problem hiding this comment.
so, is it possible to use this path for all freqs?
| @@ -1186,6 +1186,13 @@ cdef int64_t period_ordinal_to_dt64(int64_t ordinal, int freq) except? -1: | |||
| if ordinal == NPY_NAT: | |||
| return NPY_NAT | |||
|
|
|||
| if freq == 11000: | |||
| # Microsecond, avoid get_date_info to prevent floating point errors | |||
| pandas_datetime_to_datetimestruct(ordinal, NPY_FR_us, &dts) | |||
There was a problem hiding this comment.
if this path is kept, then I would do if/else here and have: L 1192, L1194 after (as they are repeated at L1197, 1198)
There was a problem hiding this comment.
k looks fine, what about higher freq than microsecond? these are ok in the get_date_info path?
There was a problem hiding this comment.
what about higher freq than microsecond? these are ok in the get_date_info path?
I've got another branch im working on to handle that; it'll end up de-duplicating some code in the process
|
ping on green |
|
updated with a fix to missing import, which I expect will be causing most CI runs to fail until it gets patched |
|
ping |
|
gentle ping @WillAyd; this is a blocker |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff