MAINT: backport Cython API cleanup to 1.17.x, remove docs by rgommers · Pull Request #14593 · numpy/numpy · GitHub

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.

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.

Seems to have been broken in some of the reorganization due to the includes changing.

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.

This document is deleted in the backport, since it is unclear and the examples don't work.

Probably not important, but some of the removed lines are still true statements.

This statement is still true on 1.17.x.

@rgommers Comment?

no idea, I have no reason to doubt @bashtage's 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?

Happy to omit this deletion, will verify later today or tomorrow and change if it still works.

OK, will wait. @bashtage makes the same comment in several places.

@rgommers Still want to look at this?

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

reverted the deletion: BitGenerator.ctypes still exists

Same comment.

reverted

Technically this statement is also True, since bit generator all export a capsule that can be unpacked in Cython get
the function pointers.

reverted the deletion

on second thought, I think that use is too obscure to be practical. Let's remove that from this release.

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).

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.

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

The current 1.17 file is renamed in this PR, do we want to do that for an existing release?

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 numpy/random/src and leaves the other in numpy/random/src/distributions. I suppose that is OK, but in 1.18 they will both be in numpy/core/include/numpy/random.

The location of the two include files differs from what is proposed for 1.18.

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!

rgommers added component: numpy.random 03 - Maintenance labels Sep 25, 2019

rgommers mentioned this pull request Sep 25, 2019

Update docs on extending numpy.random with Cython/Numba for 1.18.0 release #14594

Closed

rgommers commented Sep 25, 2019

View reviewed changes

bashtage reviewed Oct 16, 2019

View reviewed changes

rgommers added this to the 1.17.4 release. milestone Oct 31, 2019

charris modified the milestones: 1.17.4 release., 1.17.5 release Nov 11, 2019

charris added the 08 - Backport Used to tag backport PRs label Nov 12, 2019

rgommers added 3 commits November 18, 2019 10:34

DOC: remove the docs about extending random with Cython/Numba.

495e2ee

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).

DOC: add note on numpy.random Cython API removal to 1.17.3 relnotes

281ee32

mattip force-pushed the bport-cythonapi branch from a725e02 to 1b2c51a Compare November 18, 2019 17:57

MAINT: document the backport

5c0b105

mattip force-pushed the bport-cythonapi branch from 1b2c51a to 5c0b105 Compare November 18, 2019 22:19

charris reviewed Nov 18, 2019

View reviewed changes

charris merged commit a068e7d into numpy:maintenance/1.17.x Nov 19, 2019

rgommers deleted the bport-cythonapi branch November 19, 2019 02:50

charris removed this from the 1.17.5 release milestone Nov 19, 2019

               #include "numpy/npy_common.h"
               #include "numpy/npy_math.h"
-              #include "numpy/random/bitgen.h"
+              #include "src/bitgen.h"

               from libc.math cimport sqrt
-              cdef extern from "numpy/random/bitgen.h":
+              cdef extern from "src/bitgen.h":

MAINT: backport Cython API cleanup to 1.17.x, remove docs by rgommers · Pull Request #14593 · numpy/numpy · GitHub

Uh oh!

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.xnumpy/numpy:maintenance/1.17.xfrom rgommers:bport-cythonapirgommers/numpy:bport-cythonapiCopy head branch name to clipboard

Conversation

rgommers commented Sep 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bashtage left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bashtage commented Oct 24, 2019

Uh oh!

mattip commented Nov 18, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charris Nov 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charris commented Nov 19, 2019

Uh oh!

rgommers commented Nov 19, 2019

Uh oh!

charris commented Nov 19, 2019

Uh oh!

rgommers commented Nov 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MAINT: backport Cython API cleanup to 1.17.x, remove docs#14593
charris merged 4 commits into
numpy:maintenance/1.17.xfrom
rgommers:bport-cythonapi

charris Nov 18, 2019 •
edited
Loading