Refactored pandas_timedelta_to_timedeltastruct #55999
Conversation
mroeschke
added
Internals
| out->us = 0; | ||
| out->ns = 0; | ||
| } | ||
| case (NPY_FR_D): |
There was a problem hiding this comment.
is there a reason why some cases are in parens and others are not?
There was a problem hiding this comment.
No - not sure how the parentheses snuck in here. Will remove
There was a problem hiding this comment.
LGTM once these are cleaned up (and green)
| } else if (base == NPY_FR_ms) { | ||
| per_sec = 1000; | ||
| } else if (base == NPY_FR_us) { | ||
| per_sec = 1000000; |
There was a problem hiding this comment.
overlap with tslibs.dtypes.periods_per_second. probably not worth the hassle of de-duplicating
There was a problem hiding this comment.
If we wanted to we could make that an inline function like:
inline npy_int64 per_sec_multiplier(NPY_DATETIMEUNIT base):
switch base:
case NPY_FR_s:
return 1;
case NPY_FR_ms:
return 1000;
...
|
Small question, otherwise looks like a nice cleanup |
|
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
|
Sorry I let this slip - this should be reviewable now |
|
Thanks @WillAyd |
* Refactored pandas_timedelta_to_timedeltastruct * use generic macros * Revert "use generic macros" This reverts commit 29d115d. * fix sign issue * wextra fixes * more wextra * remove extraneous parantheses
this mostly cuts down on copy paste, but also implements
constwhere it can be done on some variables