MAINT: backport Cython API cleanup to 1.17.x, remove docs#14593
MAINT: backport Cython API cleanup to 1.17.x, remove docs#14593charris merged 4 commits intonumpy:maintenance/1.17.xfrom
Conversation
There was a problem hiding this comment.
Perhaps I misread this. I think the Numba example above still works so perhaps we should leave that in. Compiling distributions.c doesn't work I think, so prefer to remove that.
There was a problem hiding this comment.
Seems to have been broken in some of the reorganization due to the includes changing.
There was a problem hiding this comment.
I triple checked and it still is possible to build the DLL/so from the src dist. Not possible form the wheel though since there are no c files.
There was a problem hiding this comment.
This document is deleted in the backport, since it is unclear and the examples don't work.
There was a problem hiding this comment.
Probably not important, but some of the removed lines are still true statements.
There was a problem hiding this comment.
This statement is still true on 1.17.x.
There was a problem hiding this comment.
no idea, I have no reason to doubt @bashtage's comment
There was a problem hiding this comment.
Should we omit the deletion here, or is reasonable for 1.17 and we will cover things better in 1.18? I'd like to put this backport in. Maybe have another if there are changes in master?
There was a problem hiding this comment.
Happy to omit this deletion, will verify later today or tomorrow and change if it still works.
There was a problem hiding this comment.
OK, will wait. @bashtage makes the same comment in several places.
There was a problem hiding this comment.
sorry, failed to make time - not going to happen until end of next week at the earliest. added the 1.17.4 milestone, it seems that it should go into that release
There was a problem hiding this comment.
reverted the deletion: BitGenerator.ctypes still exists
There was a problem hiding this comment.
Technically this statement is also True, since bit generator all export a capsule that can be unpacked in Cython get
the function pointers.
There was a problem hiding this comment.
on second thought, I think that use is too obscure to be practical. Let's remove that from this release.
There was a problem hiding this comment.
Seems to have been broken in some of the reorganization due to the includes changing.
|
A quick check In [19]: import numpy as np
...: bg = np.random.PCG64()
...: bg.ctypes
...:
Out[19]: interface(state_address=140611115848248, state=c_void_p(140611115848248), next_uint64=<CFunctionType object at 0x7fe293dd6bb0>, next_uint32=<CFunctionType object at 0x7fe293dd6c80>, next_double=<CFunctionType object at 0x7fe293dd6d50>, bit_generator=c_void_p(140611115848176))shows that the cffi and the ctypes interfaces to the bit generators are still there. |
See https://mail.python.org/pipermail/numpy-discussion/2019-September/080088.html for discussion. We need to do this right, and add tests and docs. All this PR does is not install bitgen.h, common.pxd and bit_generator.pxd (they're still shipped in the sdist, that is needed). Should be backported to 1.17.x (cherry picked from commit 09ee875)
This was broken in 1.17.x and is being removed completely in 1.17.3 (to be reintroduced in 1.18.0, so this commit is not in master).
a725e02 to
1b2c51a
Compare
|
Rebased and touched up. The cffi and ctypes interfaces still are there so I left the reference. If one digs deeply they can expose the cython PyCapsule interface, but I do not think it is worth mentioning. |
1b2c51a to
5c0b105
Compare
| #include "numpy/npy_common.h" | ||
| #include "numpy/npy_math.h" | ||
| #include "numpy/random/bitgen.h" | ||
| #include "src/bitgen.h" |
There was a problem hiding this comment.
This isn't correct for 1.17, and it looks like 1.17 will be the same as 1.18 after the C-API changes go in.
charris@fc [numpy.git ((v1.17.4))]$ find . -name 'bitgen\.h'
./numpy/core/include/numpy/random/bitgen.h
There was a problem hiding this comment.
The current 1.17 file is renamed in this PR, do we want to do that for an existing release?
| from libc.math cimport sqrt | ||
|
|
||
| cdef extern from "numpy/random/bitgen.h": | ||
| cdef extern from "src/bitgen.h": |
There was a problem hiding this comment.
The original is what is in 1.17 and will also be in 1.18 after the C-API changes go in.
|
The location of the two include files differs from what is proposed for 1.18. This PR puts one in |
That's on purpose: we want the 1.17 one to be unreachable because it's not public API yet. It becomes public with cleaned up names in 1.18 |
|
Thanks Ralf, Matti. |
|
Thanks a lot @mattip for finishing this up! |
Backport of gh-14562 and removal of the docs on extending with Cython and Numba.
The Numba part says:
Both CTypes and CFFI allow the more complicated distributions to be used directly in Numba after compiling the file distributions.c into a DLL or so.
So that seems broken to and should be taken out.