More thoroughly type `*Handle` classes
- Type
*Handleclasses as much as possible - Add
.pxdfiles for*Handleobjects - Convert functions to
cpdefwhere appropriate - Convert sizes to
Py_ssize_tand use-1or0for default values - Simplify and optimize
parse_buffer_argument - Use
asarrayon the Python side to ensure we haveArrays in Cython to speedup access
There is arr.pxd in _lib. That said, point taken on the rest of them
At a first pass was thinking about internal usage and simply making that experience better
If we don't ship the .pxd, then it is not actually public for consumers of these packages. CuPy follows this practice currently ( https://github.com/cupy/cupy/issues/130 )
It is possible this could be useful externally. For example could imagine a performant Zarr implementation in Cython might use this
There is
arr.pxdin_lib. That said, point taken on the rest of them
True, we have cdef functions that takes Array so we need it in a .pxd.
At a first pass was thinking about internal usage and simply making that experience better If we don't ship the
.pxd, then it is not actually public for consumers of these packages. CuPy follows this practice currently ( cupy/cupy#130 )
Right, but I really like to have the external and class declarations close together. If we only use something in one place, it is nice to have it together in a single file IMO.
It is possible this could be useful externally. For example could imagine a performant Zarr implementation in Cython might use this
I suspect that the overhead of calling a Python vs. a Cython function is negligible at this level but it might avoid a GIL grab. I think we should wait until this becomes an issue.
Ok so is your main recommendation to fold the .pxd files back into .pyx for now?
Are there other things I should consider when doing a second pass?
Ok so is your main recommendation to fold the
.pxdfiles back into.pyxfor now?
Yes
Are there other things I should consider when doing a second pass?
I also prefer Optional[int] over a magical number like -1
Ok can look at that
The Optional[int] bit is a little odd though as we do assign default values (0) for file_offset and dev_offset, which come after. Could we make these consistent somehow? On the Cython side it would be easier if these all had default values that were ints
IMO, having 0 as a default offset argument is good because 0 is a valid argument whereas -1 is not a valid size.
Also note, none of the Cython functions has any defaults. It is the pure Python functions that introduces defaults like here so that we don't have to keep multiple defaults in sync.
I get why it is faster with int arguments but I am not sure it is worth the added complexity?