-
Notifications
You must be signed in to change notification settings - Fork 920
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
Enable Datetime/Timedelta dtypes in Masked UDFs #9451
Enable Datetime/Timedelta dtypes in Masked UDFs #9451
Conversation
cc @gmarkall |
Codecov Report
@@ Coverage Diff @@
## branch-21.12 #9451 +/- ##
================================================
+ Coverage 10.79% 10.82% +0.03%
================================================
Files 116 117 +1
Lines 18869 19451 +582
================================================
+ Hits 2036 2106 +70
- Misses 16833 17345 +512
Continue to review full report at Codecov.
|
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.
Just one question: numpy rejects operators between a datetime64
with a datetime64
type. Will numba do the same type check here?
Does numpy reject subtract operation on datetime64 vs datetime64 too? I'm aware that this is an operation supported in pandas and cudf. |
This brings up an interesting point in general. The space of allowed operations is a complex function |
Numpy does support subtraction, only rejects addition
I'm in general favor of testing the delegations when we can't have a full coverage (or full coverage cost is high), and add tests when corner cases pops up. |
This seems to be handled with this diff: import cudf
x = cudf.DataFrame({
'a': ['2011-01-01'],
'b': ['2011-02-02']
})
x['a'] = x['a'].astype('datetime64[ns]')
x['b'] = x['b'].astype('datetime64[ns]')
def f(row):
return row['a'] - row['b']
res = x.apply(f, axis=1)
added a few tests for this. |
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 looks good to me at the moment. I had one question on the diff, which probably stems from something I don't understand about datetime types.
@gpucibot merge |
Closes #9432.
Enables UDFs that do this: