-
Notifications
You must be signed in to change notification settings - Fork 116
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
SNOW-1637945: Add support for TimedeltaIndex attributes #2193
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.
LGTM.
d13f060
to
3f9806c
Compare
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.
Looks good overall. I just think we need to test against null data too.
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.
Sorry to block this now.
@sfc-gh-mvashishtha and I had a chat today about this. Ideally we should reuse the dt.properties code to do this, we should be able to support this too:
pd.Series([1,2,3]).astype("timedelta64[ns]").dt.seconds
cc @sfc-gh-helmeleegy
yeah, that is the plan. We will also use this same method to implement Series.dt.* attributes relevant for timedelta column. |
That's right. This is the plan. |
c6dfd13
to
e47d1a5
Compare
Fixes SNOW-1637945
Add support for TimedeltaIndex attributes
days
,seconds
,microseconds
, andnanoseconds
.