diff --git a/doc/python/corrade.containers.rst b/doc/python/corrade.containers.rst index 25ec9e5..42defa7 100644 --- a/doc/python/corrade.containers.rst +++ b/doc/python/corrade.containers.rst @@ -28,23 +28,26 @@ .. py:class:: corrade.containers.ArrayView - Provides one-dimensional tightly packed view on a memory range. Convertible - both from and to Python objects supporting the Buffer Protocol, with one - dimension and stride of :py:`1`. See :ref:`StridedArrayView1D` and others - for more generic views. :ref:`ArrayView` is immutable, see - :ref:`MutableArrayView` for the mutable alternative. All slicing operations - are supported, specifying a non-trivial stride will return - :ref:`StridedArrayView1D` instead of :ref:`ArrayView`. Example usage: + Provides an untyped one-dimensional read-only view on a contiguous memory + range. Convertible both to and from Python objects supporting the Buffer + Protocol. The buffer type is lost in the conversion process and the memory + is always treated as plain bytes: .. code:: pycon >>> a = b'hello' >>> b = containers.ArrayView(a) - >>> b[2] + >>> chr(b[2]) 'l' >>> bytes(b[1:4]) b'ell' + See the :ref:`StridedArrayView1D` and its multi-dimensional variants for an + alternative that can provide typed access. The :ref:`ArrayView` is + immutable, see :ref:`MutableArrayView` for the mutable alternative. All + slicing operations are supported, specifying a non-trivial stride will + return :ref:`StridedArrayView1D` instead of :ref:`ArrayView`. + `Memory ownership and reference counting`_ ========================================== @@ -79,8 +82,9 @@ .. py:class:: corrade.containers.StridedArrayView1D - Provides one-dimensional read-only view on a memory range with custom - stride values. See :ref:`StridedArrayView2D`, :ref:`StridedArrayView3D`, + Provides an one-dimensional read-only view on a memory range with custom + stride values. Convertible both to and from Python objects supporting the + Buffer Protocol. See :ref:`StridedArrayView2D`, :ref:`StridedArrayView3D`, :ref:`StridedArrayView4D`, :ref:`MutableStridedArrayView1D` and others for multi-dimensional and mutable equivalents. diff --git a/src/Corrade/Containers/StridedArrayViewPythonBindings.h b/src/Corrade/Containers/StridedArrayViewPythonBindings.h index d952c09..10d3d6d 100644 --- a/src/Corrade/Containers/StridedArrayViewPythonBindings.h +++ b/src/Corrade/Containers/StridedArrayViewPythonBindings.h @@ -38,9 +38,13 @@ template constexpr const char* pythonFormatString() { static_assert(sizeof(T) == 0, "format string unknown for this type, supply it explicitly"); return {}; } -/* Representing bytes as unsigned. Not using 'c' because then it behaves - differently from bytes/bytearray, where you can do `a[0] = ord('A')`. */ -template<> constexpr const char* pythonFormatString() { return "B"; } +/* Treating bytes as unsigned 8-bit integers and not as chars for consistency + with bytes/bytearray, where you have to use ord(a[0]) to get a character + value. Same done for PyStridedArrayViewItem and PyStridedArrayViewSetItem + below. To further emphasize that this is "general data", a null format + string is returned, which should be treated the same as B: + https://docs.python.org/3/c-api/buffer.html#c.Py_buffer.format */ +template<> constexpr const char* pythonFormatString() { return nullptr; } template<> constexpr const char* pythonFormatString() { return "b"; } template<> constexpr const char* pythonFormatString() { return "B"; } template<> constexpr const char* pythonFormatString() { return "h"; } @@ -54,6 +58,21 @@ template<> constexpr const char* pythonFormatString() { return "Q template<> constexpr const char* pythonFormatString() { return "f"; } template<> constexpr const char* pythonFormatString() { return "d"; } +template struct PyStridedArrayViewItem { + static pybind11::object get(const char* item) { + return pybind11::cast(*reinterpret_cast(item)); + } +}; +/* Treating bytes as unsigned 8-bit integers and not as chars for consistency + with bytes/bytearray, where you have to use ord(a[0]) to get a character + value. Same done for pythonFormatString() above and + PyStridedArrayViewSetItem below. */ +template<> struct PyStridedArrayViewItem { + static pybind11::object get(const char* item) { + return pybind11::cast(*reinterpret_cast(item)); + } +}; + template struct PyStridedArrayViewSetItem; template struct PyStridedArrayViewSetItem { /* __setitem__ is not even exposed for immutable views so this is fine */ @@ -64,17 +83,31 @@ template struct PyStridedArrayViewSetItem { *reinterpret_cast(item) = pybind11::cast(object); } }; +/* Treating bytes as unsigned 8-bit integers and not as chars for consistency + with bytes/bytearray, where you have to use a[0] = ord('A') to set a + character value. Same done for pythonFormatString() and + PyStridedArrayViewItem above. */ +template<> struct PyStridedArrayViewSetItem { + static void set(char* item, pybind11::handle object) { + *reinterpret_cast(item) = pybind11::cast(object); + } +}; template struct PyStridedElement; } template class PyStridedArrayView: public StridedArrayView { + /* the type is dynamic; ArrayView has the same check */ + static_assert(std::is_same::value, "only the (const) char StridedArrayView is meant to be exposed"); + public: /* Null function pointers should be okay as it shouldn't ever get to - them -- IndexError gets fired first. Not really sure about the - format, choosing bytes for safety. */ - /*implicit*/ PyStridedArrayView(): format{"B"}, getitem{}, setitem{} {} + them -- IndexError gets fired first. The format string can be null + as well (which nicely implies "general data"), in which case B + should be assumed: + https://docs.python.org/3/c-api/buffer.html#c.Py_buffer.format */ + /*implicit*/ PyStridedArrayView(): format{}, getitem{}, setitem{} {} template explicit PyStridedArrayView(const StridedArrayView& view): PyStridedArrayView{view, Implementation::pythonFormatString::type>(), sizeof(U)} {} @@ -82,9 +115,7 @@ template class PyStridedArrayView: public StridedA arrayCast(view), format, itemsize, - [](const char* item) { - return pybind11::cast(*reinterpret_cast(item)); - }, + Implementation::PyStridedArrayViewItem::get, Implementation::PyStridedArrayViewSetItem::set } {} diff --git a/src/python/corrade/containers.cpp b/src/python/corrade/containers.cpp index 7750801..e6f3bc2 100644 --- a/src/python/corrade/containers.cpp +++ b/src/python/corrade/containers.cpp @@ -109,6 +109,10 @@ template bool arrayViewBufferProtocol(T& self, Py_buffer& buffer, int f } template void arrayView(py::class_, Containers::PyArrayViewHolder>>& c) { + /* __getitem__ and __setitem__ relies on this, StridedArrayView has the + same check */ + static_assert(std::is_same::value, "only the (const) char ArrayView is meant to be exposed"); + /* Implicitly convertible from a buffer */ py::implicitly_convertible>(); /* This is needed for implicit conversion from np.array */ @@ -161,7 +165,11 @@ template void arrayView(py::class_, Containers PyErr_SetNone(PyExc_IndexError); throw py::error_already_set{}; } - return self[i]; + /* Treating bytes as unsigned 8-bit integers and not as chars for + consistency with bytes/bytearray, where you have to use + a[0] = ord('A') to set a character value. Same done for + __getitem__ and StridedArrayView. */ + return std::uint8_t(self[i]); }, "Value at given position", py::arg("i")) /* Slicing */ @@ -934,7 +942,11 @@ void containers(py::module_& m) { "MutableArrayView", "Mutable array view", py::buffer_protocol{}}; arrayView(mutableArrayView_); mutableArrayView_ - .def("__setitem__", [](const Containers::ArrayView& self, std::size_t i, const char& value) { + /* Treating bytes as unsigned 8-bit integers and not as chars for + consistency with bytes/bytearray, where you have to use + a[0] = ord('A') to set a character value. Same done for __getitem__ + and StridedArrayView. */ + .def("__setitem__", [](const Containers::ArrayView& self, std::size_t i, std::uint8_t value) { if(i >= self.size()) { PyErr_SetNone(PyExc_IndexError); throw py::error_already_set{}; diff --git a/src/python/corrade/test/test_containers.py b/src/python/corrade/test/test_containers.py index 013ce8d..cfe876e 100644 --- a/src/python/corrade/test/test_containers.py +++ b/src/python/corrade/test/test_containers.py @@ -45,23 +45,26 @@ class ArrayView(unittest.TestCase): def test_init_buffer(self): a = b'hello' a_refcount = sys.getrefcount(a) + # Verify that bytes has the same semantics, i.e. not possible to + # directly get a character + self.assertEqual(a[2], ord('l')) b = containers.ArrayView(a) self.assertIs(b.owner, a) self.assertEqual(len(b), 5) self.assertEqual(bytes(b), b'hello') - self.assertEqual(b[2], 'l') + self.assertEqual(b[2], ord('l')) self.assertEqual(sys.getrefcount(a), a_refcount + 1) # Not mutable with self.assertRaisesRegex(TypeError, "object does not support item assignment"): - b[4] = '!' + b[4] = ord('!') # b should keep a reference to a, so deleting the local reference # shouldn't affect it del a self.assertTrue(sys.getrefcount(b.owner), a_refcount) - self.assertEqual(b[2], 'l') + self.assertEqual(b[2], ord('l')) # Now, if we delete b, a should not be referenced by anything anymore a = b.owner @@ -87,9 +90,14 @@ class ArrayView(unittest.TestCase): def test_init_buffer_mutable(self): a = bytearray(b'hello') + # Verify that the bytearray has the same semantics, i.e. not possible + # to directly assign a character + a[4] = ord('?') + self.assertEqual(a[4], ord('?')) + b = containers.MutableArrayView(a) - b[4] = '!' - self.assertEqual(b[4], '!') + b[4] = ord('!') + self.assertEqual(b[4], ord('!')) self.assertEqual(bytes(b), b'hell!') def test_init_array(self): @@ -249,12 +257,17 @@ class StridedArrayView1D(unittest.TestCase): self.assertEqual(b.size, (0, )) self.assertEqual(a.stride, (0, )) self.assertEqual(b.stride, (0, )) + # By default the format is unspecified + self.assertEqual(b.format, None) self.assertEqual(a.dimensions, 1) self.assertEqual(b.dimensions, 1) def test_init_buffer(self): a = b'hello' a_refcount = sys.getrefcount(a) + # Verify that bytes has the same semantics, i.e. not possible to + # directly get a character + self.assertEqual(a[2], ord('l')) b = containers.StridedArrayView1D(a) self.assertIs(b.owner, a) @@ -262,18 +275,21 @@ class StridedArrayView1D(unittest.TestCase): self.assertEqual(bytes(b), b'hello') self.assertEqual(b.size, (5, )) self.assertEqual(b.stride, (1, )) - self.assertEqual(b[2], 'l') + # We don't provide typed access for views created from buffers, so the + # format is unspecified to convey "generic data" + self.assertEqual(b.format, None) + self.assertEqual(b[2], ord('l')) self.assertEqual(sys.getrefcount(a), a_refcount + 1) # Not mutable with self.assertRaisesRegex(TypeError, "object does not support item assignment"): - b[4] = '!' + b[4] = ord('!') # b should keep a reference to a, so deleting the local reference # shouldn't affect it del a self.assertTrue(sys.getrefcount(b.owner), a_refcount) - self.assertEqual(b[2], 'l') + self.assertEqual(b[2], ord('l')) # Now, if we delete b, a should not be referenced by anything anymore a = b.owner @@ -287,6 +303,9 @@ class StridedArrayView1D(unittest.TestCase): b = containers.StridedArrayView1D(a) self.assertIs(b.owner, None) self.assertEqual(len(b), 0) + self.assertEqual(b.size, (0, )) + self.assertEqual(b.stride, (1, )) + self.assertEqual(b.format, None) self.assertEqual(sys.getrefcount(a), a_refcount) def test_init_buffer_memoryview_obj(self): @@ -299,9 +318,20 @@ class StridedArrayView1D(unittest.TestCase): def test_init_buffer_mutable(self): a = bytearray(b'hello') + # Verify that the bytearray has the same semantics, i.e. not possible + # to directly assign a character + a[4] = ord('?') + self.assertEqual(a[4], ord('?')) + b = containers.MutableStridedArrayView1D(a) - b[4] = '!' - self.assertEqual(b[4], '!') + self.assertEqual(b.size, (5, )) + self.assertEqual(b.stride, (1, )) + # We don't provide typed access for views created from buffers, so the + # format is unspecified to convey "generic data" + self.assertEqual(b.format, None) + self.assertEqual(b[4], ord('?')) + b[4] = ord('!') + self.assertEqual(b[4], ord('!')) self.assertEqual(bytes(b), b'hell!') def test_init_buffer_unexpected_dimensions(self): @@ -318,7 +348,7 @@ class StridedArrayView1D(unittest.TestCase): self.assertEqual(bytes(b), b'hlo') self.assertEqual(b.size, (3, )) self.assertEqual(b.stride, (2, )) - self.assertEqual(b[2], 'o') + self.assertEqual(b[2], ord('o')) def test_init_buffer_mutable_from_immutable(self): a = b'hello' @@ -489,6 +519,8 @@ class StridedArrayView2D(unittest.TestCase): self.assertEqual(b.size, (0, 0)) self.assertEqual(a.stride, (0, 0)) self.assertEqual(b.stride, (0, 0)) + # By default the format is unspecified + self.assertEqual(b.format, None) self.assertEqual(a.dimensions, 2) self.assertEqual(b.dimensions, 2) @@ -508,19 +540,19 @@ class StridedArrayView2D(unittest.TestCase): self.assertEqual(b.stride, (8, 1)) self.assertIsInstance(b[1], containers.StridedArrayView1D) self.assertEqual(bytes(b[1]), b'456789ab') - self.assertEqual(b[1, 2], '6') - self.assertEqual(b[1][2], '6') + self.assertEqual(b[1, 2], ord('6')) + self.assertEqual(b[1][2], ord('6')) self.assertEqual(sys.getrefcount(a), a_refcount + 1) # Not mutable with self.assertRaisesRegex(TypeError, "object does not support item assignment"): - b[1, 2] = '!' + b[1, 2] = ord('!') # b should keep a reference to a, so deleting the local reference # shouldn't affect it del a self.assertTrue(sys.getrefcount(b.owner), a_refcount) - self.assertEqual(b[1][2], '6') + self.assertEqual(b[1][2], ord('6')) # Now, if we delete b, a should not be referenced by anything anymore a = b.owner @@ -532,10 +564,10 @@ class StridedArrayView2D(unittest.TestCase): b'456789ab' b'89abcdef') b = containers.MutableStridedArrayView2D(memoryview(a).cast('b', shape=[3, 8])) - b[0, 7] = '!' - b[1, 7] = '!' - b[2, 7] = '!' - self.assertEqual(b[0][7], '!') + b[0, 7] = ord('!') + b[1, 7] = ord('!') + b[2, 7] = ord('!') + self.assertEqual(b[0][7], ord('!')) self.assertEqual(bytes(b), b'0123456!' b'456789a!' b'89abcde!') @@ -556,7 +588,7 @@ class StridedArrayView2D(unittest.TestCase): self.assertEqual(b.size, (2, 8)) self.assertEqual(b.stride, (16, 1)) self.assertEqual(bytes(b[1]), b'89abcdef') - self.assertEqual(b[1][3], 'b') + self.assertEqual(b[1][3], ord('b')) def test_init_buffer_mutable_from_immutable(self): a = memoryview(b'01234567' @@ -814,8 +846,8 @@ class StridedArrayView3D(unittest.TestCase): self.assertEqual(bytes(b), b'01234567456789ab89abcdefcdef012301234567456789ab') self.assertEqual(b.size, (2, 3, 8)) self.assertEqual(b.stride, (24, 8, 1)) - self.assertEqual(b[1, 2, 3], '7') - self.assertEqual(b[1][2][3], '7') + self.assertEqual(b[1, 2, 3], ord('7')) + self.assertEqual(b[1][2][3], ord('7')) def test_init_buffer_mutable(self): a = bytearray(b'01234567' @@ -826,13 +858,13 @@ class StridedArrayView3D(unittest.TestCase): b'01234567' b'456789ab') b = containers.MutableStridedArrayView3D(memoryview(a).cast('b', shape=[2, 3, 8])) - b[0, 0, 7] = '!' - b[0, 1, 7] = '!' - b[0, 2, 7] = '!' - b[1, 0, 7] = '!' - b[1, 1, 7] = '!' - b[1, 2, 7] = '!' - self.assertEqual(b[1][1][7], '!') + b[0, 0, 7] = ord('!') + b[0, 1, 7] = ord('!') + b[0, 2, 7] = ord('!') + b[1, 0, 7] = ord('!') + b[1, 1, 7] = ord('!') + b[1, 2, 7] = ord('!') + self.assertEqual(b[1][1][7], ord('!')) self.assertEqual(bytes(b), b'0123456!' b'456789a!' b'89abcde!' @@ -921,8 +953,8 @@ class StridedArrayView4D(unittest.TestCase): self.assertEqual(bytes(b), b'01234567456789ab89abcdefcdef012301234567456789ab') self.assertEqual(b.size, (2, 1, 3, 8)) self.assertEqual(b.stride, (24, 24, 8, 1)) - self.assertEqual(b[1, 0, 2, 3], '7') - self.assertEqual(b[1][0][2][3], '7') + self.assertEqual(b[1, 0, 2, 3], ord('7')) + self.assertEqual(b[1][0][2][3], ord('7')) def test_init_buffer_mutable(self): a = bytearray(b'01234567' @@ -933,13 +965,13 @@ class StridedArrayView4D(unittest.TestCase): b'01234567' b'456789ab') b = containers.MutableStridedArrayView4D(memoryview(a).cast('b', shape=[2, 1, 3, 8])) - b[0, 0, 0, 7] = '!' - b[0, 0, 1, 7] = '!' - b[0, 0, 2, 7] = '!' - b[1, 0, 0, 7] = '!' - b[1, 0, 1, 7] = '!' - b[1, 0, 2, 7] = '!' - self.assertEqual(b[1][0][1][7], '!') + b[0, 0, 0, 7] = ord('!') + b[0, 0, 1, 7] = ord('!') + b[0, 0, 2, 7] = ord('!') + b[1, 0, 0, 7] = ord('!') + b[1, 0, 1, 7] = ord('!') + b[1, 0, 2, 7] = ord('!') + self.assertEqual(b[1][0][1][7], ord('!')) self.assertEqual(bytes(b), b'0123456!' b'456789a!' b'89abcde!' diff --git a/src/python/magnum/test/test.py b/src/python/magnum/test/test.py index 71e9996..1274684 100644 --- a/src/python/magnum/test/test.py +++ b/src/python/magnum/test/test.py @@ -336,8 +336,8 @@ class ImageView(unittest.TestCase): a_data = a.data self.assertEqual(len(a_data), 32) - self.assertEqual(a_data[9], 'b') - self.assertEqual(a_data[20], 'E') + self.assertEqual(a_data[9], ord('b')) + self.assertEqual(a_data[20], ord('E')) self.assertIs(a_data.owner, data) # The data references the original data as an owner, not the view self.assertEqual(sys.getrefcount(a), a_refcount) @@ -360,15 +360,15 @@ class ImageView(unittest.TestCase): a_data = a.data self.assertEqual(len(a_data), 32) - self.assertEqual(a_data[9], 'b') - self.assertEqual(a_data[20], 'E') + self.assertEqual(a_data[9], ord('b')) + self.assertEqual(a_data[20], ord('E')) self.assertIs(a_data.owner, data) # The data references the original data as an owner, not the view self.assertEqual(sys.getrefcount(a), a_refcount) self.assertEqual(sys.getrefcount(data), data_refcount + 2) - a_data[9] = '_' - a_data[20] = '_' + a_data[9] = ord('_') + a_data[20] = ord('_') self.assertEqual(data, b'rgbRGB ' b'a_cABC ' b'defD_F ' @@ -615,8 +615,8 @@ class CompressedImageView(unittest.TestCase): a_data = a.data self.assertEqual(len(a_data), 32) - self.assertEqual(a_data[9], '9') - self.assertEqual(a_data[20], 'B') + self.assertEqual(a_data[9], ord('9')) + self.assertEqual(a_data[20], ord('B')) self.assertIs(a_data.owner, data) # The data references the original data as an owner, not the view self.assertEqual(sys.getrefcount(a), a_refcount) @@ -637,15 +637,15 @@ class CompressedImageView(unittest.TestCase): a_data = a.data self.assertEqual(len(a_data), 32) - self.assertEqual(a_data[9], '9') - self.assertEqual(a_data[20], 'B') + self.assertEqual(a_data[9], ord('9')) + self.assertEqual(a_data[20], ord('B')) self.assertIs(a_data.owner, data) # The data references the original data as an owner, not the view self.assertEqual(sys.getrefcount(a), a_refcount) self.assertEqual(sys.getrefcount(data), data_refcount + 2) - a_data[9] = '_' - a_data[20] = '_' + a_data[9] = ord('_') + a_data[20] = ord('_') self.assertEqual(data, b'012345678_abcdef' b'FEDC_A9876543210') diff --git a/src/python/magnum/test/test_trade.py b/src/python/magnum/test/test_trade.py index 2228036..a24b9dd 100644 --- a/src/python/magnum/test/test_trade.py +++ b/src/python/magnum/test/test_trade.py @@ -113,7 +113,7 @@ class ImageData(unittest.TestCase): data = image.data self.assertEqual(len(data), 3*3*2) - self.assertEqual(ord(data[9 + 6 + 2]), 181) # libPNG has 12 + + self.assertEqual(data[9 + 6 + 2], 181) # libPNG has 12 + self.assertIs(data.owner, image) self.assertEqual(sys.getrefcount(image), image_refcount + 1) @@ -122,7 +122,7 @@ class ImageData(unittest.TestCase): mutable_data = image.data self.assertEqual(len(mutable_data), 3*3*2) - self.assertEqual(ord(mutable_data[9 + 6 + 2]), 181) # libPNG has 12 + + self.assertEqual(mutable_data[9 + 6 + 2], 181) # libPNG has 12 + self.assertIs(mutable_data.owner, image) self.assertEqual(sys.getrefcount(image), image_refcount + 1) @@ -139,12 +139,11 @@ class ImageData(unittest.TestCase): data = image.data mutable_data = image.mutable_data - # TODO: ugh, report as bytes, not chars - self.assertEqual(ord(data[13]), 254) - self.assertEqual(ord(mutable_data[13]), 254) + self.assertEqual(data[13], 254) + self.assertEqual(mutable_data[13], 254) - mutable_data[13] = chr(76) - self.assertEqual(data[13], chr(76)) + mutable_data[13] = 76 + self.assertEqual(data[13], 76) def test_pixels_access(self): # The only way to get an image instance is through a manager @@ -855,12 +854,11 @@ class MeshData(unittest.TestCase): index_data = mesh.index_data mutable_index_data = mesh.mutable_index_data # Second index is 2, it's a 16-bit LE number - # TODO: ugh, report as bytes, not chars - self.assertEqual(ord(index_data[2]), 2) - self.assertEqual(ord(mutable_index_data[2]), 2) + self.assertEqual(index_data[2], 2) + self.assertEqual(mutable_index_data[2], 2) - mutable_index_data[2] = chr(76) - self.assertEqual(ord(index_data[2]), 76) + mutable_index_data[2] = 76 + self.assertEqual(index_data[2], 76) def test_mutable_vertex_data_access(self): importer = trade.ImporterManager().load_and_instantiate('GltfImporter') @@ -872,12 +870,11 @@ class MeshData(unittest.TestCase): vertex_data = mesh.vertex_data mutable_vertex_data = mesh.mutable_vertex_data # The color attribute is at offset 20, G channel is the next byte - # TODO: ugh, report as bytes, not chars - self.assertEqual(ord(vertex_data[21]), 51) - self.assertEqual(ord(mutable_vertex_data[21]), 51) + self.assertEqual(vertex_data[21], 51) + self.assertEqual(mutable_vertex_data[21], 51) - mutable_vertex_data[21] = chr(76) - self.assertEqual(vertex_data[21], chr(76)) + mutable_vertex_data[21] = 76 + self.assertEqual(vertex_data[21], 76) def test_mutable_indices_access(self): importer = trade.ImporterManager().load_and_instantiate('GltfImporter')