-
-
Notifications
You must be signed in to change notification settings - Fork 25.4k
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
ENH Array API support for euclidean_distances and rbf_kernel #29433
ENH Array API support for euclidean_distances and rbf_kernel #29433
Conversation
@ogrisel Don't we have the latest version of array api strict on all CI pipelines? This seems to be failing for some pipelines probably because of the older version of array-api-strict, while it works fine on my local system with the latest version of array-api-strict. Should we fix this just for supporting an old version? |
@OmarManzoor let me update this PR with Further CI fixes will follow-up once #29436 is reviewed and merged. |
@ogrisel These CI errors seem a bit weird. One is related to the docstring of |
I thought I fixed that docstring failure somewhere else already. EDIT: here it is: @OmarManzoor feel free to steal this from the Ridge PR since I won't have time to finalize it this week. |
Please feel free to merge |
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.
I labeled "CUDA CI" to trigger a CI run with a CUDA GPU.
If everything is green, LGTM.
I tried to relabel this PR to trigger the CUDA CI out of curiosity but it's still waiting on workers for some unknown reason: |
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.
Otherwise LGTM. Thanks @ogrisel for the ping.
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. I'll let @ogrisel have another look and merge.
@ogrisel Can this be merged? |
Reference Issues/PRs
Towards #26024
What does this implement/fix? Explain your changes.
Any other comments?