From 8e8a03e175ee9445482040c298c202259341cd2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 30 Mar 2022 19:55:54 +0200 Subject: [PATCH] python: use AssertionError for trade.AbstractImporter usage errors. Because using RuntimeError conflates with import failures. Also update and fix docs to not show ValueError for where IndexError should be. --- doc/python/magnum.trade.rst | 68 +++++++++++++++------------- src/python/magnum/test/test_trade.py | 40 ++++++++-------- src/python/magnum/trade.cpp | 14 +++--- 3 files changed, 63 insertions(+), 59 deletions(-) diff --git a/doc/python/magnum.trade.rst b/doc/python/magnum.trade.rst index c6bb0d2..b9a628f 100644 --- a/doc/python/magnum.trade.rst +++ b/doc/python/magnum.trade.rst @@ -97,68 +97,72 @@ :raise RuntimeError: If file opening fails .. py:property:: magnum.trade.AbstractImporter.mesh_count - :raise RuntimeError: If no file is opened + :raise AssertionError: If no file is opened .. py:function:: magnum.trade.AbstractImporter.mesh_level_count - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than :ref:`mesh_count` + :raise AssertionError: If no file is opened + :raise IndexError: If :p:`id` is negative or not less than :ref:`mesh_count` .. py:function:: magnum.trade.AbstractImporter.mesh_for_name - :raise RuntimeError: If no file is opened + :raise AssertionError: If no file is opened .. py:function:: magnum.trade.AbstractImporter.mesh_name - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than :ref:`mesh_count` + :raise AssertionError: If no file is opened + :raise IndexError: If :p:`id` is negative or not less than :ref:`mesh_count` .. py:function:: magnum.trade.AbstractImporter.mesh - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than :ref:`mesh_count` + :raise AssertionError: If no file is opened + :raise RuntimeError: If mesh import fails + :raise IndexError: If :p:`id` is negative or not less than :ref:`mesh_count` .. py:property:: magnum.trade.AbstractImporter.image1d_count - :raise RuntimeError: If no file is opened + :raise AssertionError: If no file is opened .. py:property:: magnum.trade.AbstractImporter.image2d_count - :raise RuntimeError: If no file is opened + :raise AssertionError: If no file is opened .. py:property:: magnum.trade.AbstractImporter.image3d_count - :raise RuntimeError: If no file is opened + :raise AssertionError: If no file is opened .. py:function:: magnum.trade.AbstractImporter.image1d_level_count - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than + :raise AssertionError: If no file is opened + :raise IndexError: If :p:`id` is negative or not less than :ref:`image1d_count` .. py:function:: magnum.trade.AbstractImporter.image2d_level_count - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than + :raise AssertionError: If no file is opened + :raise IndexError: If :p:`id` is negative or not less than :ref:`image2d_count` .. py:function:: magnum.trade.AbstractImporter.image3d_level_count - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than + :raise AssertionError: If no file is opened + :raise IndexError: If :p:`id` is negative or not less than :ref:`image3d_count` .. py:function:: magnum.trade.AbstractImporter.image1d_for_name - :raise RuntimeError: If no file is opened + :raise AssertionError: If no file is opened .. py:function:: magnum.trade.AbstractImporter.image2d_for_name - :raise RuntimeError: If no file is opened + :raise AssertionError: If no file is opened .. py:function:: magnum.trade.AbstractImporter.image3d_for_name - :raise RuntimeError: If no file is opened + :raise AssertionError: If no file is opened .. py:function:: magnum.trade.AbstractImporter.image1d_name - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than + :raise AssertionError: If no file is opened + :raise IndexError: If :p:`id` is negative or not less than :ref:`image1d_count` .. py:function:: magnum.trade.AbstractImporter.image2d_name - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than + :raise AssertionError: If no file is opened + :raise IndexError: If :p:`id` is negative or not less than :ref:`image2d_count` .. py:function:: magnum.trade.AbstractImporter.image3d_name - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than + :raise AssertionError: If no file is opened + :raise IndexError: If :p:`id` is negative or not less than :ref:`image3d_count` .. py:function:: magnum.trade.AbstractImporter.image1d - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than + :raise AssertionError: If no file is opened + :raise RuntimeError: If image import fails + :raise IndexError: If :p:`id` is negative or not less than :ref:`image1d_count` .. py:function:: magnum.trade.AbstractImporter.image2d - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than + :raise AssertionError: If no file is opened + :raise RuntimeError: If image import fails + :raise IndexError: If :p:`id` is negative or not less than :ref:`image2d_count` .. py:function:: magnum.trade.AbstractImporter.image3d - :raise RuntimeError: If no file is opened - :raise ValueError: If :p:`id` is negative or not less than + :raise AssertionError: If no file is opened + :raise RuntimeError: If image import fails + :raise IndexError: If :p:`id` is negative or not less than :ref:`image3d_count` diff --git a/src/python/magnum/test/test_trade.py b/src/python/magnum/test/test_trade.py index f6e8bc5..683b96f 100644 --- a/src/python/magnum/test/test_trade.py +++ b/src/python/magnum/test/test_trade.py @@ -116,53 +116,53 @@ class Importer(unittest.TestCase): importer = trade.ImporterManager().load_and_instantiate('StbImageImporter') self.assertFalse(importer.is_opened) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.mesh_count - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.mesh_level_count(0) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.mesh_for_name('') - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.mesh_name(0) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.mesh(0) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image1d_count - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image2d_count - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image3d_count - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image1d_level_count(0) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image2d_level_count(0) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image3d_level_count(0) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image1d_for_name('') - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image2d_for_name('') - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image3d_for_name('') - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image1d_name(0) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image2d_name(0) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image3d_name(0) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image1d(0) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image2d(0) - with self.assertRaisesRegex(RuntimeError, "no file opened"): + with self.assertRaisesRegex(AssertionError, "no file opened"): importer.image3d(0) def test_index_oob(self): diff --git a/src/python/magnum/trade.cpp b/src/python/magnum/trade.cpp index 6761ce1..33a194b 100644 --- a/src/python/magnum/trade.cpp +++ b/src/python/magnum/trade.cpp @@ -143,14 +143,14 @@ template void imageData(py::class_ R checkOpened(Trade::AbstractImporter& self) { if(!self.isOpened()) { - PyErr_SetString(PyExc_RuntimeError, "no file opened"); + PyErr_SetString(PyExc_AssertionError, "no file opened"); throw py::error_already_set{}; } return (self.*f)(); } template R checkOpened(Trade::AbstractImporter& self, Arg1 arg1) { if(!self.isOpened()) { - PyErr_SetString(PyExc_RuntimeError, "no file opened"); + PyErr_SetString(PyExc_AssertionError, "no file opened"); throw py::error_already_set{}; } return (self.*f)(arg1); @@ -158,7 +158,7 @@ template R checkOpene /** @todo drop this in favor of our own string caster */ template R checkOpenedString(Trade::AbstractImporter& self, const std::string& arg1) { if(!self.isOpened()) { - PyErr_SetString(PyExc_RuntimeError, "no file opened"); + PyErr_SetString(PyExc_AssertionError, "no file opened"); throw py::error_already_set{}; } return (self.*f)(arg1); @@ -166,7 +166,7 @@ template R chec template R checkOpenedBounds(Trade::AbstractImporter& self, UnsignedInt id) { if(!self.isOpened()) { - PyErr_SetString(PyExc_RuntimeError, "no file opened"); + PyErr_SetString(PyExc_AssertionError, "no file opened"); throw py::error_already_set{}; } @@ -180,7 +180,7 @@ template std::string checkOpenedBoundsReturnsString(Trade::AbstractImporter& self, UnsignedInt id) { if(!self.isOpened()) { - PyErr_SetString(PyExc_RuntimeError, "no file opened"); + PyErr_SetString(PyExc_AssertionError, "no file opened"); throw py::error_already_set{}; } @@ -194,7 +194,7 @@ template(Trade::AbstractImporter::*f)(UnsignedInt), UnsignedInt(Trade::AbstractImporter::*bounds)() const> R checkOpenedBoundsResult(Trade::AbstractImporter& self, UnsignedInt id) { if(!self.isOpened()) { - PyErr_SetString(PyExc_RuntimeError, "no file opened"); + PyErr_SetString(PyExc_AssertionError, "no file opened"); throw py::error_already_set{}; } @@ -216,7 +216,7 @@ template(Trade::AbstractImporter::*f)(UnsignedI template(Trade::AbstractImporter::*f)(UnsignedInt, UnsignedInt), UnsignedInt(Trade::AbstractImporter::*bounds)() const, UnsignedInt(Trade::AbstractImporter::*levelBounds)(UnsignedInt)> R checkOpenedBoundsResult(Trade::AbstractImporter& self, UnsignedInt id, UnsignedInt level) { if(!self.isOpened()) { - PyErr_SetString(PyExc_RuntimeError, "no file opened"); + PyErr_SetString(PyExc_AssertionError, "no file opened"); throw py::error_already_set{}; }