From ed43b10d0e461513279bcded9d6689e0882f1f5c Mon Sep 17 00:00:00 2001 From: Antoine Lambert Date: Thu, 23 Nov 2023 01:14:03 +0100 Subject: [PATCH] talipot-python: Rework exception management in talipot python plugins When a talipot plugin written in Python is invoked from the classical Python interpreter and not from the talipot GUI, prefer to let exception that might be raised by its implementation continue its way instead of caughting it. This behavior is much more pythonic than simply printing the error traceback and clearing the python error indicator as client code calling the plugin can catch the errors it might raise. --- .../bindings/talipot-core/Algorithm.sip | 2 +- .../bindings/talipot-core/ExportModule.sip | 2 +- .../bindings/talipot-core/Graph.sip | 18 ++++++++++ .../bindings/talipot-core/ImportModule.sip | 2 +- .../bindings/talipot-core/talipot.sip | 6 ++++ .../talipot-python/modules/talipotplugins.py | 36 ++++++++----------- tests/python/test_tgf_import.py | 16 ++++++--- 7 files changed, 54 insertions(+), 28 deletions(-) diff --git a/library/talipot-python/bindings/talipot-core/Algorithm.sip b/library/talipot-python/bindings/talipot-core/Algorithm.sip index 9f95c8c75f..219117701d 100644 --- a/library/talipot-python/bindings/talipot-core/Algorithm.sip +++ b/library/talipot-python/bindings/talipot-core/Algorithm.sip @@ -50,7 +50,7 @@ public: virtual ~Algorithm(); - virtual bool run() = 0; + virtual bool run() = 0 /VirtualErrorHandler=pass_through/; %Docstring tlp.Algorithm.run() diff --git a/library/talipot-python/bindings/talipot-core/ExportModule.sip b/library/talipot-python/bindings/talipot-core/ExportModule.sip index e8499bea18..51c7737ab0 100644 --- a/library/talipot-python/bindings/talipot-core/ExportModule.sip +++ b/library/talipot-python/bindings/talipot-core/ExportModule.sip @@ -87,7 +87,7 @@ saves to. virtual std::string category() const; - virtual bool exportGraph(std::ostream &os) = 0; + virtual bool exportGraph(std::ostream &os) = 0 /VirtualErrorHandler=pass_through/; %Docstring tlp.ExportModule.exportGraph(os) diff --git a/library/talipot-python/bindings/talipot-core/Graph.sip b/library/talipot-python/bindings/talipot-core/Graph.sip index 8b7e05babf..1338b11eb2 100644 --- a/library/talipot-python/bindings/talipot-core/Graph.sip +++ b/library/talipot-python/bindings/talipot-core/Graph.sip @@ -84,6 +84,9 @@ bool callGraphPropertyAlgorithm(tlp::Graph *graph, const std::string &algoName, msg += "."; PyErr_SetString(PyExc_Exception, msg.c_str()); } + if (PyErr_Occurred()) { + sipIsErr = -1; + } return ret; } @@ -357,6 +360,9 @@ if (pluginExists(*a0)) { msg += "."; PyErr_SetString(PyExc_Exception, msg.c_str()); } +if (PyErr_Occurred()) { + sipIsErr = -1; +} %End //=========================================================================================== @@ -5267,6 +5273,9 @@ if (pluginExists(*a0)) { msg += "."; PyErr_SetString(PyExc_Exception, msg.c_str()); } +if (PyErr_Occurred()) { + sipIsErr = -1; +} %End tlp::Graph *importGraph(const std::string &importPluginName, tlp::DataSet dataSet /GetWrapper/ ) /TransferBack/; @@ -5318,6 +5327,9 @@ if (pluginExists(*a0)) { msg += "."; PyErr_SetString(PyExc_Exception, msg.c_str()); } +if (PyErr_Occurred()) { + sipIsErr = -1; +} %End //=========================================================================================== @@ -5373,6 +5385,9 @@ if (pluginExists(*a0)) { msg += "."; PyErr_SetString(PyExc_Exception, msg.c_str()); } +if (PyErr_Occurred()) { + sipIsErr = -1; +} %End //=========================================================================================== @@ -5441,6 +5456,9 @@ if (pluginExists(*a0)) { msg += "."; PyErr_SetString(PyExc_Exception, msg.c_str()); } +if (PyErr_Occurred()) { + sipIsErr = -1; +} %End //=========================================================================================== diff --git a/library/talipot-python/bindings/talipot-core/ImportModule.sip b/library/talipot-python/bindings/talipot-core/ImportModule.sip index 8ba7cd9358..0048b653ee 100644 --- a/library/talipot-python/bindings/talipot-core/ImportModule.sip +++ b/library/talipot-python/bindings/talipot-core/ImportModule.sip @@ -87,7 +87,7 @@ This method returns all the extensions (normal and gzipped) of file formats this list of string %End - virtual bool importGraph() = 0; + virtual bool importGraph() = 0 /VirtualErrorHandler=pass_through/; %Docstring tlp.ImportModule.importGraph() diff --git a/library/talipot-python/bindings/talipot-core/talipot.sip b/library/talipot-python/bindings/talipot-core/talipot.sip index 71252ba342..4f9c7b32c0 100644 --- a/library/talipot-python/bindings/talipot-core/talipot.sip +++ b/library/talipot-python/bindings/talipot-core/talipot.sip @@ -32,6 +32,12 @@ PyRun_SimpleString("import sys;globals()['talipot'] = sys.modules['talipot']"); %End +%VirtualErrorHandler pass_through +// pass through virtual error handler used to not clear Python error indicator +// in some Python implementation of C++ virtual methods +%End + + %ModuleHeaderCode #include #include diff --git a/library/talipot-python/modules/talipotplugins.py b/library/talipot-python/modules/talipotplugins.py index 69b68a8901..2e48321089 100644 --- a/library/talipot-python/modules/talipotplugins.py +++ b/library/talipot-python/modules/talipotplugins.py @@ -84,15 +84,13 @@ def runPlugin(plugin): except Exception: if plugin.pluginProgress: plugin.pluginProgress.setError(traceback.format_exc()) - # Case where the plugin execution has not been launched through the - # Talipot GUI, so print the stack trace to stderr from talipot import tlp if type(plugin.pluginProgress) == tlp.SimplePluginProgress: - sys.stdout.write(('There was an error when running Python plugin ' - 'named "%s". See stack trace below.\n') % - plugin.name()) - traceback.print_exc() - return ret + # case where the plugin execution has not been launched through the + # Talipot GUI, raise exception again in that case + raise + finally: + return ret def importGraph(plugin): @@ -102,15 +100,13 @@ def importGraph(plugin): except Exception: if plugin.pluginProgress: plugin.pluginProgress.setError(traceback.format_exc()) - # Case where the plugin execution has not been launched through - # the Talipot GUI, so print the stack trace to stderr from talipot import tlp if type(plugin.pluginProgress) == tlp.SimplePluginProgress: - sys.stdout.write(('There was an error when running Python plugin ' - 'named "%s". See stack trace below.\n') % - plugin.name()) - traceback.print_exc() - return ret + # case where the plugin execution has not been launched through the + # Talipot GUI, raise exception again in that case + raise + finally: + return ret def exportGraph(plugin, os): @@ -120,15 +116,13 @@ def exportGraph(plugin, os): except Exception: if plugin.pluginProgress: plugin.pluginProgress.setError(traceback.format_exc()) - # Case where the plugin execution has not been launched through - # the Talipot GUI, so print the stack trace to stderr from talipot import tlp if type(plugin.pluginProgress) == tlp.SimplePluginProgress: - sys.stdout.write(('There was an error when running Python plugin ' - 'named "%s". See stack trace below.\n') % - plugin.name()) - traceback.print_exc() - return ret + # case where the plugin execution has not been launched through the + # Talipot GUI, raise exception again in that case + raise + finally: + return ret def createPlugin(context, pluginModule, pluginClassName, pluginName, diff --git a/tests/python/test_tgf_import.py b/tests/python/test_tgf_import.py index ebfb39c988..1ec7f28edb 100644 --- a/tests/python/test_tgf_import.py +++ b/tests/python/test_tgf_import.py @@ -109,8 +109,12 @@ def test_load_tgf_graph_invalid_edge(self): """ ) ) - graph = tlp.importGraph(self.tgfPluginName, {"filename": tgfFile}) - self.assertTrue(graph is None) + with self.assertRaises(ValueError) as cm: + tlp.importGraph(self.tgfPluginName, {"filename": tgfFile}) + self.assertEquals( + cm.exception.args[0], + "Invalid edge definition: 4" + ) def test_load_tgf_graph_invalid_node_id(self): with tempfile.TemporaryDirectory() as tgfDir: @@ -128,5 +132,9 @@ def test_load_tgf_graph_invalid_node_id(self): """ ) ) - graph = tlp.importGraph(self.tgfPluginName, {"filename": tgfFile}) - self.assertTrue(graph is None) + with self.assertRaises(ValueError) as cm: + tlp.importGraph(self.tgfPluginName, {"filename": tgfFile}) + self.assertEquals( + cm.exception.args[0], + "Invalid node identifier found when parsing edges: 4" + )