Convert .pxi include files to .pxd/.pyx in dpnp/tensor#2913
Convert .pxi include files to .pxd/.pyx in dpnp/tensor#2913vlad-perevezentsev wants to merge 9 commits into
Conversation
|
Array API standard conformance tests for dpnp=0.21.0dev0=py313h509198e_41 ran successfully. |
|
View rendered docs @ https://intelpython.github.io/dpnp/pull/2913/index.html |
| include "_stride_utils.pxi" | ||
| include "_types.pxi" | ||
| include "_slicing.pxi" | ||
| from ._slicing cimport * |
There was a problem hiding this comment.
Is it a good practice to use star cimport?
| from cpython.buffer cimport PyObject_CheckBuffer | ||
| from numpy import ndarray | ||
|
|
||
| from ._usmarray cimport usm_ndarray |
There was a problem hiding this comment.
Does not that produce a circular dependency? (_usmarray ↔ _slicing)
|
|
||
|
|
||
| # Local storage for `cdef public api` constants | ||
| # declared in _usmarray.pxd |
There was a problem hiding this comment.
Is there a way to avoid redefinition? To keep the constant defined in single place, the same is applicable to UAR_* constants.
There was a problem hiding this comment.
when these files were in dpctl, my thought was to move these constants into another, separate .pxd which could just be included by both
There was a problem hiding this comment.
@ndgrigorian could you clarify what you mean by a separate .pxd file?
Previously these constants were defined in .pxi files that were included directly into _usmarray.pyx which provided local storage for cdef public api
cimport from another .pxd does not work that (header is generated but constants are zero at runtime)
There was a problem hiding this comment.
@ndgrigorian could you clarify what you mean by a separate
.pxdfile? Previously these constants were defined in.pxifiles that were included directly into_usmarray.pyxwhich provided local storage forcdef public apicimportfrom another.pxddoes not work that (header is generated but constants are zero at runtime)
I see, so reason for this is subtle
cython/cython#4369
here is Cython issue which discusses it, found by googling this issue.
The best approach, in this case, is probably to separate the constants to a .h header file which can then be cdef extern from, and then that can also be used in dpnp4pybind11 to set the constants for the CAPI
There was a problem hiding this comment.
we could also use the enum approach, if preferred, but I think the header approach is cleaner
This PR proposes to replace Cython textual includes with proper module imports in
dpnp/tensor.pxi files are converted into corresponding .pxd and .pyx modules for _slicing.pxi, _stride_utils.pxi and _types.pxi`
This change replaces the include
.pxipattern with standard Cython cimport statements and improving modularity