-
Notifications
You must be signed in to change notification settings - Fork 157
Update exchange API to be capsule following new convention #180
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
base: main
Are you sure you want to change the base?
Conversation
|
cc @Kathryn-cat |
|
RFC thread #179 |
This PR updates the exchange API to be capsule following the new convention. Note that the exchange API python recommendation is not yet updated on the array-api side and so far there hasn't yet been downstream implementation, so this change should be safe in generally.
|
Array api spec PR data-apis/array-api#984 |
|
Sorry for delay, will try to finish my review in a day or two! |
leofang
left a comment
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.
Thanks, @tqchen! LGTM overall, left some questions that we should address before merging.
| direct interaction between the array framework PyObject* and DLPack, | ||
| as a result it is harder to implement the C Exchange API through ctypes (because | ||
| ctypes releases GIL by default and sometimes in non-free-threading environment, | ||
| GIL is needed to interact with the Python C API). |
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.
Co-authored-by: Leo Fang <[email protected]>
Co-authored-by: Leo Fang <[email protected]>
|
@leofang please take another look, all comments are addressed |
This PR updates the exchange API to be capsule following the new convention. Note that the exchange API python recommendation is not yet updated on the array-api side and so far there hasn't yet been downstream implementation, so this change should be safe in generally.