-
Notifications
You must be signed in to change notification settings - Fork 879
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
[REVIEW]Handle index when dispatching __array_function__ and __array_ufunc__ to cupy for cudf.Series #6839
[REVIEW]Handle index when dispatching __array_function__ and __array_ufunc__ to cupy for cudf.Series #6839
Conversation
Please update the changelog in order to start CI tests. View the gpuCI docs here. |
Codecov Report
@@ Coverage Diff @@
## branch-0.17 #6839 +/- ##
===============================================
+ Coverage 81.94% 81.95% +0.01%
===============================================
Files 96 96
Lines 16164 16168 +4
===============================================
+ Hits 13246 13251 +5
+ Misses 2918 2917 -1
Continue to review full report at Codecov.
|
expect = np.concatenate([ar, ar, ar]) | ||
got = np.concatenate([s, s, s]) | ||
assert_eq(expect, got.to_array()) | ||
with pytest.raises(TypeError): |
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.
Removed this as the index is undefined when dispatching to cupy
for these functions.
rerun tests |
In the process of looking this over - I am building this branch to better understand the problem being solved here. |
Below might help. With this PR we get the correct index on the following: >>> cudf_s1 = cudf.Series(data=[-1, 2, 3, 0], index=[2, 3, 1, 0])
>>> cudf_s2 = cudf.Series(data=[-1, -2, 3, 0], index=[2, 3, 1, 0])
>>> o = np.logaddexp(cudf_s1, cudf_s2)
>>> o.index
Int64Index([2, 3, 1, 0], dtype='int64')
>>> print(o)
2 -0.306853
3 2.018150
1 3.693147
0 0.693147
dtype: float64 On Master we get:>>> cudf_s1 = cudf.Series(data=[-1, 2, 3, 0], index=[2, 3, 1, 0])
>>> cudf_s2 = cudf.Series(data=[-1, -2, 3, 0], index=[2, 3, 1, 0])
>>> o = np.logaddexp(cudf_s1, cudf_s2)
>>> o.index
RangeIndex(start=0, stop=4, step=1)
>>> print(o)
0 -0.306853
1 2.018150
2 3.693147
3 0.693147
dtype: float64 |
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 some general questions.
All reviews have been addressed, this should be ready for another review. CC: @brandon-b-miller , @isVoid |
One last question otherwise LGTM. |
@isVoid, Please let me know if there is anything else I need to address. |
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!
This pr adds index handling when dispatching to cupy functions with
__ufunc__
and__array_function__
for cudf.Series.This PR does the following:
__ufunc__
and__array_function
(when being dispatched tocupy
)list
inputs (should not have been supported initially too)Please note that I am unsure how to handle
list
inputs here.The problem being solved here is below:
With this PR #6839 we get the correct index when we do the following:
On Master we get: