Browse Source

python: slightly improve perf of __getitem__ on matrices/vectors.

Also test them properly. The full optimization is waiting for a pybind
PR to get merged.
pull/2/head
Vladimír Vondruš 7 years ago
parent
commit
22a17cb5b8
  1. 25
      src/python/magnum/math.matrix.h
  2. 15
      src/python/magnum/math.vector.h
  3. 16
      src/python/magnum/test/test_math.py

25
src/python/magnum/math.matrix.h

@ -164,23 +164,38 @@ template<class T> void rectangularMatrix(py::class_<T>& c) {
.def(py::self != py::self, "Non-equality comparison")
/* Set / get. Need to throw IndexError in order to allow iteration:
https://docs.python.org/3/reference/datamodel.html#object.__getitem__ */
https://docs.python.org/3/reference/datamodel.html#object.__getitem__
Using error_already_set is slightly faster than throwing index_error
directly, but still much slower than not throwing at all. Waiting
for https://github.com/pybind/pybind11/pull/1853 to get merged. */
.def("__setitem__", [](T& self, std::size_t i, const typename VectorTraits<T::Rows, typename T::Type>::Type& value) {
if(i >= T::Cols) throw pybind11::index_error{};
if(i >= T::Cols) {
PyErr_SetString(PyExc_IndexError, "");
throw pybind11::error_already_set{};
}
self[i] = value;
}, "Set a column at given position")
.def("__getitem__", [](const T& self, std::size_t i) -> typename VectorTraits<T::Rows, typename T::Type>::Type {
if(i >= T::Cols) throw pybind11::index_error{};
if(i >= T::Cols) {
PyErr_SetString(PyExc_IndexError, "");
throw pybind11::error_already_set{};
}
return self[i];
}, "Column at given position")
/* Set / get for direct elements, because [a][b] = 2.5 won't work
without involving shared pointers */
.def("__setitem__", [](T& self, const std::pair<std::size_t, std::size_t>& i, typename T::Type value) {
if(i.first >= T::Cols || i.second >= T::Rows) throw pybind11::index_error{};
if(i.first >= T::Cols || i.second >= T::Rows) {
PyErr_SetString(PyExc_IndexError, "");
throw pybind11::error_already_set{};
}
self[i.first][i.second] = value;
}, "Set a value at given col/row")
.def("__getitem__", [](const T& self, const std::pair<std::size_t, std::size_t>& i) {
if(i.first >= T::Cols || i.second >= T::Rows) throw pybind11::index_error{};
if(i.first >= T::Cols || i.second >= T::Rows) {
PyErr_SetString(PyExc_IndexError, "");
throw pybind11::error_already_set{};
}
return self[i.first][i.second];
}, "Value at given col/row")

15
src/python/magnum/math.vector.h

@ -191,13 +191,22 @@ template<class T> void vector(py::module& m, py::class_<T>& c) {
.def(py::self >= py::self, "Component-wise greater than or equal comparison")
/* Set / get. Need to throw IndexError in order to allow iteration:
https://docs.python.org/3/reference/datamodel.html#object.__getitem__ */
https://docs.python.org/3/reference/datamodel.html#object.__getitem__
Using error_already_set is slightly faster than throwing index_error
directly, but still much slower than not throwing at all. Waiting
for https://github.com/pybind/pybind11/pull/1853 to get merged. */
.def("__setitem__", [](T& self, std::size_t i, typename T::Type value) {
if(i >= T::Size) throw pybind11::index_error{};
if(i >= T::Size) {
PyErr_SetString(PyExc_IndexError, "");
throw pybind11::error_already_set{};
}
self[i] = value;
}, "Set a value at given position")
.def("__getitem__", [](const T& self, std::size_t i) {
if(i >= T::Size) throw pybind11::index_error{};
if(i >= T::Size) {
PyErr_SetString(PyExc_IndexError, "");
throw pybind11::error_already_set{};
}
return self[i];
}, "Value at given position")

16
src/python/magnum/test/test_math.py

@ -241,6 +241,12 @@ class Vector(unittest.TestCase):
b.xy += (1, -1)
self.assertEqual(b, Vector4i(4, 3, 15, 6))
# OOB access should fire, as that's needed for iteration
with self.assertRaises(IndexError):
a[5]
with self.assertRaises(IndexError):
a[3] = 1.1
def test_iterate(self):
a = [i for i in Vector4(1.0, 3.25, 3.5, -1.125)]
# assertAlmostEqual doesn't work on lists so i'm using values directly
@ -560,6 +566,16 @@ class Matrix(unittest.TestCase):
a[0, 1] = 2.5
self.assertEqual(a[0], Vector3(1.0, 2.5, 3.0)) # yes, 2.5
# OOB access should fire, as that's needed for iteration
with self.assertRaises(IndexError):
a[2]
with self.assertRaises(IndexError):
a[1, 3]
with self.assertRaises(IndexError):
a[2] = Vector3()
with self.assertRaises(IndexError):
a[2, 1] = 0.5
@unittest.expectedFailure
def test_set_two_brackets(self):
a = Matrix2x3((1.0, 2.0, 3.0),

Loading…
Cancel
Save