From 24ef90db63a5a6f2a91c7d3c30285ab0133e0fb8 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Sun, 29 May 2022 10:46:17 -0700 Subject: API: Retain `arr.base` more strictly in `np.frombuffer` Checking the implementation of `releasebuffer` is the most strict version for when we can safely assume that wrapping into a `memoryview` is not necessary. In theory, `view.obj` could also be set to a new object even with the old buffer-protocol, in practice I don't think that should happen (the docs read like it should not be used). So this is the minimal fix to retain old behavior as much as (safely) possible. Closes gh-21612 --- numpy/core/src/multiarray/ctors.c | 17 +++++++++-------- numpy/core/tests/test_multiarray.py | 13 +++++++------ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c index c5a7ebf7d..bbd73e280 100644 --- a/numpy/core/src/multiarray/ctors.c +++ b/numpy/core/src/multiarray/ctors.c @@ -3685,15 +3685,16 @@ PyArray_FromBuffer(PyObject *buf, PyArray_Descr *type, } /* - * The array check is probably unnecessary. It preserves the base for - * arrays. This is the "old" buffer protocol, which had no release logic. - * (It was assumed that the result is always a view.) - * - * NOTE: We could also check if `bf_releasebuffer` is defined which should - * be the most precise and safe thing to do. But that should only be - * necessary if unexpected backcompat issues are found downstream. + * If the object supports `releasebuffer`, the new buffer protocol allows + * tying the memories lifetime to the `Py_buffer view`. + * NumPy cannot hold on to the view itself (it is not an object) so it + * has to wrap the original object in a Python `memoryview` which deals + * with the lifetime management for us. + * For backwards compatibility of `arr.base` we try to avoid this when + * possible. (For example, NumPy arrays will never get wrapped here!) */ - if (!PyArray_Check(buf)) { + if (Py_TYPE(buf)->tp_as_buffer + && Py_TYPE(buf)->tp_as_buffer->bf_releasebuffer) { buf = PyMemoryView_FromObject(buf); if (buf == NULL) { return NULL; diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index 35acf307f..deb706752 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -5350,12 +5350,13 @@ class TestFromBuffer: buf = x.tobytes() assert_array_equal(np.frombuffer(buf, dtype=dt), x.flat) - def test_array_base(self): - arr = np.arange(10) - new = np.frombuffer(arr) - # We currently special case arrays to ensure they are used as a base. - # This could probably be changed (removing the test). - assert new.base is arr + @pytest.mark.parametrize("obj", [np.arange(10), b"12345678"]) + def test_array_base(self, obj): + # Objects (including NumPy arrays), which do not use the + # `release_buffer` slot should be directly used as a base object. + # See also gh-21612 + new = np.frombuffer(obj) + assert new.base is obj def test_empty(self): assert_array_equal(np.frombuffer(b''), np.array([])) -- cgit v1.2.1