python: make ArrayView element access behavior match bytes/bytearray.

In the buffer protocol it used to advertise untyped data with B as the
format string, but the __getitem__ and __setitem__ were using the char
type (implicitly coming from the fact that the type exposed is
ArrayView<char>, StridedArrayViewND<char> or their const variants),
resulting in the data being treated as characters by Python. Which
was extremely annoying and inconsistent with how the bytes and bytearray
behaves.

Now ArrayView bindings always operate with std::uint8_t, and for
StridedArrayView there's a special case for the <char> type, which makes
it treated as std::uint8_t as well. Furthermore, to hint that the <char>
is "general data", the format string for it is null / None instead of 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.
 

src/Corrade/Containers/StridedArrayViewPythonBindings.h

@@ -38,9 +38,13 @@ template<class T> 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<char>() { 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<char>() { return nullptr; }
 template<> constexpr const char* pythonFormatString<std::int8_t>() { return "b"; }
 template<> constexpr const char* pythonFormatString<std::uint8_t>() { return "B"; }
 template<> constexpr const char* pythonFormatString<std::int16_t>() { return "h"; }
@@ -54,6 +58,21 @@ template<> constexpr const char* pythonFormatString<std::uint64_t>() { return "Q
 template<> constexpr const char* pythonFormatString<float>() { return "f"; }
 template<> constexpr const char* pythonFormatString<double>() { return "d"; }
 
+template<class U> struct PyStridedArrayViewItem {
+    static pybind11::object get(const char* item) {
+        return pybind11::cast(*reinterpret_cast<const U*>(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<char>() above and
+   PyStridedArrayViewSetItem below. */
+template<> struct PyStridedArrayViewItem<const char> {
+    static pybind11::object get(const char* item) {
+        return pybind11::cast(*reinterpret_cast<const std::uint8_t*>(item));
+    }
+};
+
 template<class T, class U> struct PyStridedArrayViewSetItem;
 template<class U> struct PyStridedArrayViewSetItem<const char, U> {
     /* __setitem__ is not even exposed for immutable views so this is fine */
@@ -64,17 +83,31 @@ template<class U> struct PyStridedArrayViewSetItem<char, U> {
         *reinterpret_cast<U*>(item) = pybind11::cast<U>(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<char>() and
+   PyStridedArrayViewItem above. */
+template<> struct PyStridedArrayViewSetItem<char, char> {
+    static void set(char* item, pybind11::handle object) {
+        *reinterpret_cast<std::uint8_t*>(item) = pybind11::cast<std::uint8_t>(object);
+    }
+};
 
 template<unsigned, class> struct PyStridedElement;
 
 }
 
 template<unsigned dimensions, class T> class PyStridedArrayView: public StridedArrayView<dimensions, T> {
+    /* the type is dynamic; ArrayView has the same check */
+    static_assert(std::is_same<const T, const char>::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<class U> explicit PyStridedArrayView(const StridedArrayView<dimensions, U>& view): PyStridedArrayView{view, Implementation::pythonFormatString<typename std::decay<U>::type>(), sizeof(U)} {}
 
@@ -82,9 +115,7 @@ template<unsigned dimensions, class T> class PyStridedArrayView: public StridedA
             arrayCast<T>(view),
             format,
             itemsize,
-            [](const char* item) {
-                return pybind11::cast(*reinterpret_cast<const U*>(item));
-            },
+            Implementation::PyStridedArrayViewItem<const U>::get,
             Implementation::PyStridedArrayViewSetItem<T, U>::set
         } {}
 

src/python/corrade/containers.cpp

@@ -109,6 +109,10 @@ template<class T> bool arrayViewBufferProtocol(T& self, Py_buffer& buffer, int f
 }
 
 template<class T> void arrayView(py::class_<Containers::ArrayView<T>, Containers::PyArrayViewHolder<Containers::ArrayView<T>>>& c) {
+    /* __getitem__ and __setitem__ relies on this, StridedArrayView has the
+       same check */
+    static_assert(std::is_same<const T, const char>::value, "only the (const) char ArrayView is meant to be exposed");
+
     /* Implicitly convertible from a buffer */
     py::implicitly_convertible<py::buffer, Containers::ArrayView<T>>();
     /* This is needed for implicit conversion from np.array */
@@ -161,7 +165,11 @@ template<class T> void arrayView(py::class_<Containers::ArrayView<T>, 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<char>& 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<char>& self, std::size_t i, std::uint8_t value) {
             if(i >= self.size()) {
                 PyErr_SetNone(PyExc_IndexError);
                 throw py::error_already_set{};

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!'

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')
 

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')