Skip to content

Commit

Permalink
talipot-python: Rework exception management in talipot python plugins
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
anlambert committed Nov 23, 2023
1 parent 0706a36 commit ed43b10
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 28 deletions.
2 changes: 1 addition & 1 deletion library/talipot-python/bindings/talipot-core/Algorithm.sip
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public:

virtual ~Algorithm();

virtual bool run() = 0;
virtual bool run() = 0 /VirtualErrorHandler=pass_through/;
%Docstring
tlp.Algorithm.run()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
18 changes: 18 additions & 0 deletions library/talipot-python/bindings/talipot-core/Graph.sip
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -357,6 +360,9 @@ if (pluginExists<tlp::Algorithm>(*a0)) {
msg += ".";
PyErr_SetString(PyExc_Exception, msg.c_str());
}
if (PyErr_Occurred()) {
sipIsErr = -1;
}
%End

//===========================================================================================
Expand Down Expand Up @@ -5267,6 +5273,9 @@ if (pluginExists<tlp::ImportModule>(*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/;
Expand Down Expand Up @@ -5318,6 +5327,9 @@ if (pluginExists<tlp::ImportModule>(*a0)) {
msg += ".";
PyErr_SetString(PyExc_Exception, msg.c_str());
}
if (PyErr_Occurred()) {
sipIsErr = -1;
}
%End

//===========================================================================================
Expand Down Expand Up @@ -5373,6 +5385,9 @@ if (pluginExists<tlp::ImportModule>(*a0)) {
msg += ".";
PyErr_SetString(PyExc_Exception, msg.c_str());
}
if (PyErr_Occurred()) {
sipIsErr = -1;
}
%End

//===========================================================================================
Expand Down Expand Up @@ -5441,6 +5456,9 @@ if (pluginExists<tlp::ExportModule>(*a0)) {
msg += ".";
PyErr_SetString(PyExc_Exception, msg.c_str());
}
if (PyErr_Occurred()) {
sipIsErr = -1;
}
%End

//===========================================================================================
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
6 changes: 6 additions & 0 deletions library/talipot-python/bindings/talipot-core/talipot.sip
Original file line number Diff line number Diff line change
Expand Up @@ -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 <talipot/Graph.h>
#include <talipot/PluginsManager.h>
Expand Down
36 changes: 15 additions & 21 deletions library/talipot-python/modules/talipotplugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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,
Expand Down
16 changes: 12 additions & 4 deletions tests/python/test_tgf_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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"
)

0 comments on commit ed43b10

Please sign in to comment.