Skip to content
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

ENH: resync numpy/__init__.pxd with upstream #16170

Merged
merged 3 commits into from May 12, 2020
Merged

Conversation

mattip
Copy link
Member

@mattip mattip commented May 6, 2020

As part of the fall-out of gh-16162, @scoder suggested we remove the buffer protocol shims from our __init__.pxd. As long as I was touching it, I added nogil to the macros and removed a few non-public functions.

@mattip
Copy link
Member Author

mattip commented May 6, 2020

I am pretty sure numpy.random excercises much of this pxd file, so did not add more tests.

scoder added a commit to cython/cython that referenced this pull request May 6, 2020
…the GIL (according to the declarations shipped by NumPy itself).

See numpy/numpy#16170
scoder added a commit to cython/cython that referenced this pull request May 6, 2020
@scoder
Copy link
Contributor

scoder commented May 6, 2020

I added the same nogil declarations in Cython.

scoder added a commit to cython/cython that referenced this pull request May 7, 2020
…es and document them (copied from NumPy docs).

Adapt the 'nogil' declarations according to the ones in NumPy and add some missing functions.
See numpy/numpy#16170
@scoder
Copy link
Contributor

scoder commented May 7, 2020

Synced in Cython. I also turned all remaining struct field declarations into properties, descr and base. That makes them read-only as well – let's see who complains about the next alpha release. :)

@mattip mattip added this to the 1.19.0 release milestone May 7, 2020
int PyArray_INCREF (ndarray)
int PyArray_XDECREF (ndarray)
# int PyArray_INCREF (ndarray)
# int PyArray_XDECREF (ndarray)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious about the removal of these two functions, and the two below. They are part of the API, does cython not want to use them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, thanks. Reverting this change. @scoder: fyi

@@ -477,16 +470,16 @@ cdef extern from "numpy/arrayobject.h":
npy_intp PyArray_REFCOUNT(object)
object PyArray_ContiguousFromAny(op, int, int min_depth, int max_depth)
unsigned char PyArray_EquivArrTypes(ndarray a1, ndarray a2)
bint PyArray_EquivByteorders(int b1, int b2)
bint PyArray_EquivByteorders(int b1, int b2) nogil
object PyArray_SimpleNew(int nd, npy_intp* dims, int typenum)
object PyArray_SimpleNewFromData(int nd, npy_intp* dims, int typenum, void* data)
#object PyArray_SimpleNewFromDescr(int nd, npy_intp* dims, dtype descr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or rather, why was it removed previously.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see that there a lot of these commented out without comment. It would be nice to know why, but that is not something for this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't say. The git history says Update numpy.pxd when these were added :)
Feel free to uncomment them, then we should follow in Cython. (Happy to receive a PR :) )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to expand a bit on the comments above: this file was copied from Cython, where the functions were already commented out (the few I checked were last touched in 2009).

@charris charris merged commit 823c59c into numpy:master May 12, 2020
@charris
Copy link
Member

charris commented May 12, 2020

Let's put this in. It would be nice to hear back from @scoder, but I don't think we need to wait. Thanks Matti.

@mattip mattip deleted the cleanup-pxd branch April 8, 2021 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants