Skip to content

DOC: document PyArray_ResolveWritebackIfCopy #10166

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

Merged
merged 3 commits into from
Dec 6, 2017

Conversation

mattip
Copy link
Member

@mattip mattip commented Dec 6, 2017

missing from PR #9639, assumes UPDATEIFCOPY will be deprecated

@mattip mattip mentioned this pull request Dec 6, 2017
@@ -3251,6 +3252,15 @@ Memory management
:c:data:`NPY_USE_PYMEM` is 0, if :c:data:`NPY_USE_PYMEM` is 1, then
the Python memory allocator is used.

.. c:function:: PyArray_ResolveWritebackIfCopy(PyArrayObject* obj)
Copy link
Member

Choose a reason for hiding this comment

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

What is the return value?

@@ -1357,9 +1357,10 @@ Special functions for NPY_OBJECT
.. c:function:: int PyArray_SetWritebackIfCopyBase(PyArrayObject* arr, PyArrayObject* base)
Copy link
Member

@charris charris Dec 6, 2017

Choose a reason for hiding this comment

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

The return value also needs documentation.

@charris
Copy link
Member

charris commented Dec 6, 2017

My impression was that the deprecation warning was only active when run with PyPy or NumPy was compiled with a special flag. Is that incorrect?

@charris
Copy link
Member

charris commented Dec 6, 2017

I think the example in c-info.howto-extend also needs to be marked with a Numpy version and a comment about PyPy compatibility. In fact, it might be helpful to use #if PYPY_VERSION in the code to show both traditional and new versions of the code. Something like

#ifdef PYPY_VERSION
/* PyPy compatible version for Numpy >= 1.14.0 */
...
#else
/* Traditional version using ... */

@mattip
Copy link
Member Author

mattip commented Dec 6, 2017

Using NPY_ARRAY_UPDATEIFCOPY will always emit a ``DeprecationWarning` when creating the array but never when deallocating it.

Using NPY_ARRAY_WRITEBACKIFCOPY or PyArray_SetWritebackIfCopy() will emit a DeprecationWarning when deallocating the array unless PyArray_ResolveWritebackIfCopy is called.

Only on PyPy (or with the flag) using PyArray_SetUpdateIfCopy will emit a warning DeprecationWarning but never when deallocating it.

Edit: correct flag names

@charris
Copy link
Member

charris commented Dec 6, 2017

Using NPY_UPDATEIFCOPY will always emit a ``DeprecationWarning` when creating the array but never when deallocating it.

This is the one that worries me as there is no fallback for code intended to work with earlier NumPy versions. SciPy uses it for instance. Also, the official macro name is NPY_ARRAY_UPDATEIFCOPY, the other alias is out of date (scipy uses both and needs a fix).

@mattip
Copy link
Member Author

mattip commented Dec 6, 2017

SciPy has already updated for the new behavior scipy/scipy#8150

@charris
Copy link
Member

charris commented Dec 6, 2017

I suppose we could leave it as is and deal with it later if there are complaints.

@mattip
Copy link
Member Author

mattip commented Dec 6, 2017

Thanks, sorry for not getting everyone on board earlier

@charris
Copy link
Member

charris commented Dec 6, 2017

No problem. Merged, thanks @mattip .

@charris charris merged commit 0058e70 into numpy:master Dec 6, 2017
@mattip mattip deleted the doc-PyArray_ResolveWritebackIfCopy branch December 6, 2017 21:53
@charris
Copy link
Member

charris commented Dec 6, 2017

I may have been a bit quick here, shouldn't the example be using NPY_ARRAY_INOUT_ARRAY2?

@mattip
Copy link
Member Author

mattip commented Dec 6, 2017

good catch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants