Skip to content

Turn all cdef class objects in the Python layer to cdef public class #464

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

Open
leofang opened this issue Feb 24, 2025 · 4 comments
Open
Labels
cuda.bindings Everything related to the cuda.bindings module enhancement Any code-related improvements P1 Medium priority - Should do triage Needs the team's attention

Comments

@leofang
Copy link
Member

leofang commented Feb 24, 2025

This allows us to share the class declarations with C/C++:
https://cython.readthedocs.io/en/latest/src/userguide/external_C_code.html#using-cython-declarations-from-c
which is needed after PR #463 set up a path for Python-C/C++/Cython interoperability, but without adding the public qualifier the interoperability was limited to Cython only.

@leofang leofang added cuda.bindings Everything related to the cuda.bindings module enhancement Any code-related improvements P1 Medium priority - Should do labels Feb 24, 2025
@leofang
Copy link
Member Author

leofang commented Feb 24, 2025

@vzhurba01 I hope this is a simple one-liner change to the codegen and the remaining task is on verifying the generated headers. Could you take a look?

@leofang leofang added the triage Needs the team's attention label Feb 24, 2025
@leofang
Copy link
Member Author

leofang commented Apr 23, 2025

It is unclear to me if we still need this done if we want to go by the route of #564. @oleksandr-pavlyk @shwina WDYT?

@shwina
Copy link
Contributor

shwina commented Apr 23, 2025

Hmm, if my understanding is correct, the two seem a bit orthogonal:

  1. cdef public would allow e.g., third-party C/C++ code to use types defined in Cython (not sure if we know of use cases for this)
  2. get_native_hande() would allow us to grab a pointer to the underlying native CUDA object wrapped by a Cython type

Am I missing something or can (1) be used in place of (2) somehow?

@oleksandr-pavlyk
Copy link
Contributor

oleksandr-pavlyk commented Apr 23, 2025

@leofang, I think it should be cdef api instead of cdef public.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda.bindings Everything related to the cuda.bindings module enhancement Any code-related improvements P1 Medium priority - Should do triage Needs the team's attention
Projects
None yet
Development

No branches or pull requests

3 participants