diff --git a/doc/python/corrade.containers.rst b/doc/python/corrade.containers.rst index cdcf146..02f4e74 100644 --- a/doc/python/corrade.containers.rst +++ b/doc/python/corrade.containers.rst @@ -51,26 +51,16 @@ Unlike in C++, the view keeps a reference to the original memory owner object, meaning that calling :py:`del` on the original object will *not* invalidate the view. Slicing a view creates a new view referencing the same - original object, without any dependency on the previous view. + original object, without any dependency on the previous view. That means a + long chained slicing operation will not cause increased memory usage. .. code:: pycon >>> b.obj is a True - >>> b[1:4].obj is a + >>> b[1:4][:-1].obj is a True - .. block-danger:: Pybind11 Buffer Protocol issues - - Currently, due to how buffer protocol is exposed in pybind11, there are - several issues with converting array views from and to buffer objects: - - - readonly property is not preserved when converting an ArrayView - to Python `memoryview` - - subsequent slicing and view conversion keeps reference to the - previous view instance instead of just to the original object, - creating unnecesarily long GC chains - `Comparison to Python's memoryview`_ ==================================== diff --git a/src/python/corrade/PyArrayView.h b/src/python/corrade/PyArrayView.h index 3b2731c..24f6c4f 100644 --- a/src/python/corrade/PyArrayView.h +++ b/src/python/corrade/PyArrayView.h @@ -38,6 +38,10 @@ template struct PyArrayView: Containers::ArrayView { /*implicit*/PyArrayView() noexcept: obj{py::none{}} {} explicit PyArrayView(Containers::ArrayView view, py::object obj) noexcept: Containers::ArrayView{view}, obj{std::move(obj)} {} + /* Turning this into a reference so buffer protocol can point to it instead + of needing to allocate */ + std::size_t& sizeRef() { return Containers::ArrayView::_size; } + py::object obj; }; @@ -46,6 +50,15 @@ template struct PyStridedArrayView: Containers::St /*implicit*/ PyStridedArrayView() noexcept: obj{py::none{}} {} explicit PyStridedArrayView(Containers::StridedArrayView view, py::object obj) noexcept: Containers::StridedArrayView{view}, obj{std::move(obj)} {} + /* Turning this into a reference so buffer protocol can point to it instead + of needing to allocate */ + Containers::StridedDimensions& sizeRef() { + return Containers::StridedArrayView::_size; + } + Containers::StridedDimensions& strideRef() { + return Containers::StridedArrayView::_stride; + } + py::object obj; }; diff --git a/src/python/corrade/PyBuffer.h b/src/python/corrade/PyBuffer.h new file mode 100644 index 0000000..5e51d3c --- /dev/null +++ b/src/python/corrade/PyBuffer.h @@ -0,0 +1,82 @@ +#ifndef corrade_PyBuffer_h +#define corrade_PyBuffer_h +/* + This file is part of Magnum. + + Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 + Vladimír Vondruš + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included + in all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. +*/ + +#include +#include +#include + +#include "bootstrap.h" + +namespace corrade { + +/* pybind's py::buffer_info is EXTREMELY USELESS IT HURTS (and also allocates + like hell), doing my own thing here instead. IMAGINE, I can pass flags to + say what features I'm able to USE! WOW! */ + +template void enableBetterBufferProtocol(py::object& object) { + auto& typeObject = reinterpret_cast(*object.ptr()); + /* Sanity check -- we expect pybind set up its own buffer functions before + us */ + CORRADE_INTERNAL_ASSERT(typeObject.as_buffer.bf_getbuffer == pybind11::detail::pybind11_getbuffer); + CORRADE_INTERNAL_ASSERT(typeObject.as_buffer.bf_releasebuffer == pybind11::detail::pybind11_releasebuffer); + + typeObject.as_buffer.bf_getbuffer = [](PyObject *obj, Py_buffer *buffer, int flags) { + /* Stolen from pybind11::class_::def_buffer(). Not sure what exactly + it does, but I assume caster is implicitly convertible to Class& and + thus can magically access the actual Class from the PyObject. */ + pybind11::detail::make_caster caster; + CORRADE_INTERNAL_ASSERT(!PyErr_Occurred() && buffer); + CORRADE_INTERNAL_ASSERT(caster.load(obj, /*convert=*/false)); + + /* Zero-initialize the output and ask the class to fill it. If that + fails for some reason, give up */ + *buffer = Py_buffer{}; + if(!getter(caster, *buffer, flags)) { + CORRADE_INTERNAL_ASSERT(!buffer->obj); + CORRADE_INTERNAL_ASSERT(PyErr_Occurred()); + return -1; + } + + /* Set the memory owner to the object and increase its reference count. + We need to keep the object around because buffer->shapes / + buffer->strides might be refering to it, moreover setting it to + something else (like ArrayView's memory owner object) would mean + Python calls the releasebuffer on that object instead of on us, + leading to reference count getting negative in many cases. */ + CORRADE_INTERNAL_ASSERT(!buffer->obj); + buffer->obj = obj; + Py_INCREF(buffer->obj); + return 0; + }; + /* No need to release anything, we haven't made any garbage in the first + place */ + typeObject.as_buffer.bf_releasebuffer = nullptr; +} + +} + +#endif diff --git a/src/python/corrade/containers.cpp b/src/python/corrade/containers.cpp index c8306a1..85fe372 100644 --- a/src/python/corrade/containers.cpp +++ b/src/python/corrade/containers.cpp @@ -26,16 +26,26 @@ #include #include /* so ArrayView is convertible from python array */ #include +#include #include #include "corrade/bootstrap.h" #include "corrade/PyArrayView.h" #include "corrade/PybindExtras.h" +#include "corrade/PyBuffer.h" namespace corrade { namespace { +const char* const FormatStrings[]{ + /* 0. Representing bytes as unsigned. Not using 'c' because then it behaves + differently from bytes/bytearray, where you can do `a[0] = ord('A')`. */ + "B", +}; +template constexpr std::size_t formatIndex(); +template<> constexpr std::size_t formatIndex() { return 0; } + struct Slice { std::size_t start; std::size_t stop; @@ -61,6 +71,32 @@ Slice calculateSlice(py::slice slice, std::size_t containerSize) { return Slice{std::size_t(start), std::size_t(stop), step}; } +template bool arrayViewBufferProtocol(T& self, Py_buffer& buffer, int flags) { + if((flags & PyBUF_WRITABLE) == PyBUF_WRITABLE && !std::is_const::value) { + PyErr_SetString(PyExc_BufferError, "array view is not writable"); + return false; + } + + /* I hate the const_casts but I assume this is to make editing easier, NOT + to make it possible for users to stomp on these values. */ + buffer.ndim = 1; + buffer.itemsize = sizeof(typename T::Type); + buffer.len = sizeof(typename T::Type)*self.size(); + buffer.buf = const_cast::type*>(self.data()); + buffer.readonly = std::is_const::value; + if((flags & PyBUF_FORMAT) == PyBUF_FORMAT) + buffer.format = const_cast(FormatStrings[formatIndex::type>()]); + if(flags != PyBUF_SIMPLE) { + /* The view is immutable (can't change its size after it has been + constructed), so referencing the size directly is okay */ + buffer.shape = reinterpret_cast(&self.sizeRef()); + if((flags & PyBUF_STRIDES) == PyBUF_STRIDES) + buffer.strides = &buffer.itemsize; + } + + return true; +} + template void arrayView(py::class_>& c) { /* Implicitly convertible from a buffer */ py::implicitly_convertible>(); @@ -70,27 +106,25 @@ template void arrayView(py::class_>& c) { .def(py::init(), "Default constructor") /* Buffer protocol */ - .def(py::init([](py::buffer buffer) { - py::buffer_info info = buffer.request(!std::is_const::value); - - if(info.ndim != 1) - throw py::buffer_error{Utility::formatString("expected one dimension but got {}", info.ndim)}; - if(info.strides[0] != info.itemsize) - throw py::buffer_error{Utility::formatString("expected stride of {} but got {}", info.itemsize, info.strides[0])}; - - // TODO: need to take buffer.obj, not buffer! - return PyArrayView{{static_cast(info.ptr), std::size_t(info.shape[0]*info.itemsize)}, buffer}; + .def(py::init([](py::buffer other) { + Py_buffer buffer{}; + if(PyObject_GetBuffer(other.ptr(), &buffer, (std::is_const::value ? 0 : PyBUF_WRITABLE)) != 0) + throw py::error_already_set{}; + + Containers::ScopeGuard e{&buffer, PyBuffer_Release}; + + if(buffer.ndim != 1) + throw py::buffer_error{Utility::formatString("expected one dimension but got {}", buffer.ndim)}; + if(buffer.strides && buffer.strides[0] != buffer.itemsize) + throw py::buffer_error{Utility::formatString("expected stride of {} but got {}", buffer.itemsize, buffer.strides[0])}; + + /* reinterpret_borrow converts PyObject* to an (automatically + refcounted) py::object. We take the underlying object instead of + the buffer because we no longer care about the buffer + descriptor -- that could allow the GC to haul away a bit more + garbage */ + return PyArrayView{{static_cast(buffer.buf), std::size_t(buffer.len)}, py::reinterpret_borrow(buffer.obj)}; }), "Construct from a buffer") - .def_buffer([](const PyArrayView& self) -> py::buffer_info { - return py::buffer_info{ - const_cast(self.data()), - sizeof(T), - py::format_descriptor::format(), - 1, - {self.size()}, - {1} // TODO: need to pass self.obj to the buffer, not self! - }; - }) /* Length and memory owning object */ .def("__len__", &PyArrayView::size, "View size") @@ -121,6 +155,8 @@ template void arrayView(py::class_>& c) { /* Usual business */ return py::cast(PyArrayView{self.slice(calculated.start, calculated.stop), self.obj}); }, "Slice the view"); + + enableBetterBufferProtocol, arrayViewBufferProtocol>(c); } template void mutableArrayView(py::class_>& c) { @@ -202,42 +238,76 @@ template const T& dimensionsTupleGet(const typename DimensionsTuple<3, CORRADE_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ } +template bool stridedArrayViewBufferProtocol(T& self, Py_buffer& buffer, int flags) { + if((flags & PyBUF_STRIDES) != PyBUF_STRIDES) { + /* TODO: allow this if the array actually *is* contiguous? */ + PyErr_SetString(PyExc_BufferError, "array view is not contiguous"); + return false; + } + + if((flags & PyBUF_WRITABLE) == PyBUF_WRITABLE && !std::is_const::value) { + PyErr_SetString(PyExc_BufferError, "array view is not writable"); + return false; + } + + /* I hate the const_casts but I assume this is to make editing easier, NOT + to make it possible for users to stomp on these values. */ + buffer.ndim = T::Dimensions; + buffer.itemsize = sizeof(typename T::Type); + buffer.len = sizeof(typename T::Type); + for(std::size_t i = 0; i != T::Dimensions; ++i) + buffer.len *= self.sizeRef()[i]; + buffer.buf = const_cast::type*>(self.data()); + buffer.readonly = std::is_const::value; + if((flags & PyBUF_FORMAT) == PyBUF_FORMAT) + buffer.format = const_cast(FormatStrings[formatIndex::type>()]); + /* The view is immutable (can't change its size after it has been + constructed), so referencing the size/stride directly is okay */ + buffer.shape = const_cast(reinterpret_cast(self.sizeRef().begin())); + buffer.strides = const_cast(reinterpret_cast(self.strideRef().begin())); + + return true; +} + +inline std::size_t largerStride(std::size_t a, std::size_t b) { + return a < b ? b : a; /* max(), but named like this to avoid clashes */ +} + template void stridedArrayView(py::class_>& c) { c /* Constructor */ .def(py::init(), "Default constructor") /* Buffer protocol */ - .def(py::init([](py::buffer buffer) { - py::buffer_info info = buffer.request(!std::is_const::value); - - if(info.ndim != dimensions) - throw py::buffer_error{Utility::formatString("expected {} dimensions but got {}", dimensions, info.ndim)}; - + .def(py::init([](py::buffer other) { + Py_buffer buffer{}; + if(PyObject_GetBuffer(other.ptr(), &buffer, PyBUF_STRIDES|(std::is_const::value ? 0 : PyBUF_WRITABLE)) != 0) + throw py::error_already_set{}; + + Containers::ScopeGuard e{&buffer, PyBuffer_Release}; + + if(buffer.ndim != dimensions) + throw py::buffer_error{Utility::formatString("expected {} dimensions but got {}", dimensions, buffer.ndim)}; + + Containers::StaticArrayView sizes{reinterpret_cast(buffer.shape)}; + Containers::StaticArrayView strides{reinterpret_cast(buffer.strides)}; + /* Calculate total memory size that spans the whole view. Mainly to + make the constructor assert happy, not used otherwise */ + std::size_t size = 0; + for(std::size_t i = 0; i != dimensions; ++i) + size = largerStride(buffer.shape[i]*(buffer.strides[i] < 0 ? -buffer.strides[i] : buffer.strides[i]), size); + + /* reinterpret_borrow converts PyObject* to an (automatically + refcounted) py::object. We take the underlying object instead of + the buffer because we no longer care about the buffer + descriptor -- that could allow the GC to haul away a bit more + garbage */ return PyStridedArrayView{{ - {static_cast(info.ptr), std::size_t(info.size*info.strides[0])}, - Containers::StaticArrayView{reinterpret_cast(&info.shape[0])}, - Containers::StaticArrayView{reinterpret_cast(&info.strides[0])}}, - // TODO: need to take buffer.obj, not buffer! - buffer}; + {static_cast(buffer.buf), size}, + Containers::StaticArrayView{reinterpret_cast(buffer.shape)}, + Containers::StaticArrayView{reinterpret_cast(buffer.strides)}}, + py::reinterpret_borrow(buffer.obj)}; }), "Construct from a buffer") - .def_buffer([](const PyStridedArrayView& self) -> py::buffer_info { - std::vector shape(dimensions); - Containers::StridedDimensions selfSize{self.size()}; - for(std::size_t i = 0; i != dimensions; ++i) shape[i] = selfSize[i]; - - std::vector strides(dimensions); - Containers::StridedDimensions selfStride{self.stride()}; - for(std::size_t i = 0; i != dimensions; ++i) strides[i] = selfStride[i]; - return py::buffer_info{ - const_cast(self.data()), - sizeof(T), - py::format_descriptor::format(), - dimensions, - shape, strides - // TODO: need to pass self.obj to the buffer, not self! - }; - }) // TODO: construct from a buffer + size/stride @@ -266,6 +336,8 @@ template void stridedArrayView(py::class_{self.size()}[0]); return py::cast(PyStridedArrayView(self.slice(calculated.start, calculated.stop).every(calculated.step), self.obj)); }, "Slice the view"); + + enableBetterBufferProtocol, stridedArrayViewBufferProtocol>(c); } template void stridedArrayView1D(py::class_>& c) { diff --git a/src/python/corrade/test/test_containers.py b/src/python/corrade/test/test_containers.py index beb784c..ac89edd 100644 --- a/src/python/corrade/test/test_containers.py +++ b/src/python/corrade/test/test_containers.py @@ -66,6 +66,14 @@ class ArrayView(unittest.TestCase): del b self.assertTrue(sys.getrefcount(a), a_refcount) + def test_init_buffer_memoryview_obj(self): + a = b'hello' + v = memoryview(a) + b = containers.ArrayView(v) + # memoryview's buffer protocol returns itself, not the underlying + # bytes, as it manages the Py_buffer instance. So this is expected. + self.assertIs(b.obj, v) + def test_init_buffer_mutable(self): a = bytearray(b'hello') b = containers.MutableArrayView(a) @@ -79,16 +87,18 @@ class ArrayView(unittest.TestCase): self.assertIs(b.obj, a) self.assertEqual(len(b), 3*4) + @unittest.skip("there doesn't seem to be a way to make memoryview give back N-dimensional array that can't be represented as linear memory") def test_init_buffer_unexpected_dimensions(self): a = memoryview(b'123456').cast('b', shape=[2, 3]) self.assertEqual(bytes(a), b'123456') - with self.assertRaisesRegex(BufferError, "expected one dimension but got 2"): + with self.assertRaisesRegex(BufferError, "but what"): b = containers.ArrayView(a) def test_init_buffer_unexpected_stride(self): a = memoryview(b'hello')[::2] self.assertEqual(bytes(a), b'hlo') - with self.assertRaisesRegex(BufferError, "expected stride of 1 but got 2"): + # Error emitted by memoryview, not us + with self.assertRaisesRegex(BufferError, "memoryview: underlying buffer is not C-contiguous"): b = containers.ArrayView(a) def test_init_buffer_mutable_from_immutable(self): @@ -183,12 +193,24 @@ class ArrayView(unittest.TestCase): b_refcount = sys.getrefcount(b) self.assertEqual(sys.getrefcount(a), a_refcount + 1) - # TODO: fix when pybind is replaced - c = memoryview(b) - self.assertEqual(c.obj, b) # TODO: should be a - self.assertEqual(sys.getrefcount(b), b_refcount + 1) # TODO: should not hcange - self.assertEqual(sys.getrefcount(a), a_refcount + 1) # TODO: should be +2 + # Unlike slicing, ArrayView's buffer protocol returns a reference to + # itself -- it needs to be kept around because the Py_buffer refers to + # its internals for size. Also returning a reference to the underlying + # buffer would mean the underlying buffer's releasebuffer function gets + # called instead of ours which is *not* wanted. + self.assertEqual(c.obj, b) + self.assertEqual(sys.getrefcount(b), b_refcount + 1) + self.assertEqual(sys.getrefcount(a), a_refcount + 1) + + with self.assertRaisesRegex(TypeError, "cannot modify read-only memory"): + c[-1] = ord('?') + + def test_convert_mutable_memoryview(self): + a = bytearray(b'World is hell!') + b = memoryview(containers.MutableArrayView(a)) + b[-1] = ord('?') + self.assertEqual(a, b'World is hell?') class StridedArrayView1D(unittest.TestCase): def test_init(self): @@ -235,12 +257,13 @@ class StridedArrayView1D(unittest.TestCase): del b self.assertTrue(sys.getrefcount(a), a_refcount) - @unittest.expectedFailure def test_init_buffer_memoryview_obj(self): a = b'hello' v = memoryview(a) b = containers.StridedArrayView1D(v) - self.assertIs(b.obj, a) # TODO: it's b because pybind is stupid + # memoryview's buffer protocol returns itself, not the underlying + # bytes, as it manages the Py_buffer instance. So this is expected. + self.assertIs(b.obj, v) def test_init_buffer_mutable(self): a = bytearray(b'hello') @@ -334,18 +357,28 @@ class StridedArrayView1D(unittest.TestCase): b_refcount = sys.getrefcount(b) self.assertEqual(sys.getrefcount(a), a_refcount + 1) - # TODO: fix when pybind is replaced - c = memoryview(b) self.assertEqual(c.ndim, 1) self.assertEqual(len(c), len(a)) self.assertEqual(bytes(c), a) - self.assertEqual(c.obj, b) # TODO: should be a - self.assertEqual(sys.getrefcount(b), b_refcount + 1) # TODO: should not change - self.assertEqual(sys.getrefcount(a), a_refcount + 1) # TODO: should be +2 + # Unlike slicing, StridedArrayView's buffer protocol returns a + # reference to itself and not the underlying buffer -- it needs to be + # kept around because the Py_buffer refers to its internals for size. + # Also returning a reference to the underlying buffer would mean the + # underlying buffer's releasebuffer function gets called instead of + # ours which is *not* wanted. + self.assertEqual(c.obj, b) + self.assertEqual(sys.getrefcount(b), b_refcount + 1) + self.assertEqual(sys.getrefcount(a), a_refcount + 1) - c[-1] = ord('?') # TODO: wrong, should fail - self.assertEqual(a, b'World is hell?') # TODO: wrong + with self.assertRaisesRegex(TypeError, "cannot modify read-only memory"): + c[-1] = ord('?') + + def test_convert_mutable_memoryview(self): + a = bytearray(b'World is hell!') + b = memoryview(containers.MutableStridedArrayView1D(a)) + b[-1] = ord('?') + self.assertEqual(a, b'World is hell?') class StridedArrayView2D(unittest.TestCase): def test_init(self): @@ -370,9 +403,9 @@ class StridedArrayView2D(unittest.TestCase): b'89abcdef') a_refcount = sys.getrefcount(a) - b = containers.StridedArrayView2D(memoryview(a).cast('b', shape=[3, 8])) + v = memoryview(a).cast('b', shape=[3, 8]) # TODO: construct as containers.StridedArrayView2D(a, (3, 8), (8, 1)) - #self.assertIs(b.obj, a) # TODO + b = containers.StridedArrayView2D(v) self.assertEqual(len(b), 3) self.assertEqual(bytes(b), b'01234567' b'456789ab' @@ -439,20 +472,17 @@ class StridedArrayView2D(unittest.TestCase): b = containers.MutableStridedArrayView2D(a) def test_slice(self): - a = (b'01234567' - b'456789ab' - b'89abcdef') + a = memoryview(b'01234567' + b'456789ab' + b'89abcdef').cast('b', shape=[3, 8]) a_refcount = sys.getrefcount(a) - v = memoryview(a).cast('b', shape=[3, 8]) - v_refcount = sys.getrefcount(v) - - # TODO: Pybind refcounts against v, not a - - b = containers.StridedArrayView2D(v) + b = containers.StridedArrayView2D(a) b_refcount = sys.getrefcount(b) + # memoryview's buffer protocol returns itself, not the underlying + # bytes, as it manages the Py_buffer instance. So this is expected. + self.assertEqual(b.obj, a) self.assertEqual(sys.getrefcount(a), a_refcount + 1) - self.assertEqual(sys.getrefcount(v), v_refcount + 1) # TODO: should not change # When slicing, b's refcount should not change but a's refcount should # increase @@ -462,30 +492,25 @@ class StridedArrayView2D(unittest.TestCase): self.assertEqual(c.stride, (8, 1)) self.assertEqual(bytes(c), b'01234567456789ab') self.assertEqual(sys.getrefcount(b), b_refcount) - self.assertEqual(sys.getrefcount(a), a_refcount + 1) # TODO: should be +2 - self.assertEqual(sys.getrefcount(v), v_refcount + 2) # TODO: should not change + self.assertEqual(sys.getrefcount(a), a_refcount + 2) # Deleting a slice should reduce a's refcount again, keep b's unchanged del c self.assertEqual(sys.getrefcount(b), b_refcount) self.assertEqual(sys.getrefcount(a), a_refcount + 1) - self.assertEqual(sys.getrefcount(v), v_refcount + 1) # TODO: should not change def test_slice_multidimensional(self): - a = (b'01234567' - b'456789ab' - b'89abcdef') + a = memoryview(b'01234567' + b'456789ab' + b'89abcdef').cast('b', shape=[3, 8]) a_refcount = sys.getrefcount(a) - v = memoryview(a).cast('b', shape=[3, 8]) - v_refcount = sys.getrefcount(v) - - # TODO: Pybind refcounts against v, not a - - b = containers.StridedArrayView2D(v) + b = containers.StridedArrayView2D(a) b_refcount = sys.getrefcount(b) + # memoryview's buffer protocol returns itself, not the underlying + # bytes, as it manages the Py_buffer instance. So this is expected. + self.assertEqual(b.obj, a) self.assertEqual(sys.getrefcount(a), a_refcount + 1) - self.assertEqual(sys.getrefcount(v), v_refcount + 1) # TODO: should not change # When slicing, b's refcount should not change but a's refcount should # increase @@ -496,14 +521,12 @@ class StridedArrayView2D(unittest.TestCase): self.assertEqual(bytes(c[0]), b'89a') self.assertEqual(bytes(c[1]), b'cde') self.assertEqual(sys.getrefcount(b), b_refcount) - self.assertEqual(sys.getrefcount(a), a_refcount + 1) # TODO: should be +2 - self.assertEqual(sys.getrefcount(v), v_refcount + 2) # TODO: should not change + self.assertEqual(sys.getrefcount(a), a_refcount + 2) # Deleting a slice should reduce a's refcount again, keep b's unchanged del c self.assertEqual(sys.getrefcount(b), b_refcount) self.assertEqual(sys.getrefcount(a), a_refcount + 1) - self.assertEqual(sys.getrefcount(v), v_refcount + 1) # TODO: should not change def test_slice_invalid(self): with self.assertRaisesRegex(ValueError, "slice step cannot be zero"): @@ -600,29 +623,25 @@ class StridedArrayView2D(unittest.TestCase): self.assertEqual(bytes(d), b'3377bb') def test_convert_memoryview(self): - a = (b'01234567' - b'456789ab' - b'89abcdef') + a = memoryview(b'01234567' + b'456789ab' + b'89abcdef').cast('b', shape=[3, 8]) a_refcount = sys.getrefcount(a) - v = memoryview(a).cast('b', shape=[3, 8]) - v_refcount = sys.getrefcount(v) - b = containers.StridedArrayView2D(v) + b = containers.StridedArrayView2D(a) b_refcount = sys.getrefcount(b) - self.assertEqual(sys.getrefcount(a), a_refcount + 1) # TODO: should be + 2 - self.assertEqual(sys.getrefcount(v), v_refcount + 1) # TODO: should not change + # memoryview's buffer protocol returns itself, not the underlying + # bytes, as it manages the Py_buffer instance. So this is expected. + self.assertEqual(b.obj, a) + self.assertEqual(sys.getrefcount(a), a_refcount + 1) c = memoryview(b) self.assertEqual(c.ndim, 2) self.assertEqual(c.shape, (3, 8)) self.assertEqual(c.strides, (8, 1)) - self.assertEqual(c.obj, v) # TODO: should be a - self.assertEqual(sys.getrefcount(b), b_refcount + 1) # TODO: should not change - self.assertEqual(sys.getrefcount(a), a_refcount + 1) # TODO: should be + 2 - self.assertEqual(sys.getrefcount(v), v_refcount + 1) # TODO: should not change - - c[2, 1] = ord('!') # TODO: wrong, should fail - self.assertEqual(chr(c[2, 1]), '!') # TODO: should be 9 + self.assertEqual(c.obj, a) + self.assertEqual(sys.getrefcount(a), a_refcount + 1) + self.assertEqual(sys.getrefcount(b), b_refcount + 1) class StridedArrayView3D(unittest.TestCase): def test_init_buffer(self):