Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace C++ exit(1) statements with exception handling #329

Merged
merged 8 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion efel/cppcore/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ add_library(efel SHARED ${FEATURESRCS})
install(TARGETS efel LIBRARY DESTINATION lib)

install(FILES efel.h cfeature.h FillFptrTable.h LibV1.h LibV2.h LibV3.h
LibV5.h mapoperations.h Utils.h DependencyTree.h eFELLogger.h
LibV5.h mapoperations.h Utils.h DependencyTree.h eFELLogger.h EfelExceptions.h
types.h
DESTINATION include)
12 changes: 12 additions & 0 deletions efel/cppcore/EfelExceptions.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#ifndef EFEL_EXCEPTIONS_H
#define EFEL_EXCEPTIONS_H

#include <stdexcept>

// Define the custom exception class
class EfelAssertionError : public std::runtime_error {
public:
explicit EfelAssertionError(const std::string& message) : std::runtime_error(message) {}
};

#endif // EFEL_EXCEPTIONS_H
6 changes: 4 additions & 2 deletions efel/cppcore/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <numeric>
#include <vector>
#include <utility>
#include "EfelExceptions.h"

using std::vector;

Expand Down Expand Up @@ -85,8 +86,9 @@ inline void
efel_assert(bool assertion, const char *message, const char *file, const int line)
{
if(!assertion){
printf("Assertion fired(%s:%d): %s\n", file, line, message);
exit(-1);
using std::string, std::to_string;
string errorMsg = "Assertion fired(" + string(file) + ":" + to_string(line) + "): " + string(message);
throw EfelAssertionError(errorMsg);
}
}

Expand Down
29 changes: 8 additions & 21 deletions efel/cppcore/cfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -374,14 +374,9 @@ int cFeature::calc_features(const string& name) {
map<string, vector<featureStringPair> >::const_iterator lookup_it(
fptrlookup.find(name));
if (lookup_it == fptrlookup.end()) {
fprintf(stderr,
"\nFeature [ %s ] dependency file entry or pointer table entry is "
"missing. Exiting\n",
name.c_str());
fflush(stderr);
exit(1);
throw std::runtime_error("Feature dependency file entry or pointer table "
"entry for '" + name + "' is missing.");
}

bool last_failed = false;

for (vector<featureStringPair>::const_iterator pfptrstring =
Expand Down Expand Up @@ -581,17 +576,12 @@ double cFeature::getDistance(string strName, double mean, double std,
}
}

// check datatype of feature
featureType = featuretype(strName);
if (featureType.empty()) {
printf("Error : Feature [%s] not found. Exiting..\n", strName.c_str());
exit(1);
}

if (featureType == "int") {
retVal = getFeature<int>(strName, feature_veci);
intFlag = 1;
} else {
} else { // double
retVal = getFeature<double>(strName, feature_vec);
intFlag = 0;
}
Expand Down Expand Up @@ -633,14 +623,11 @@ double cFeature::getDistance(string strName, double mean, double std,
}

string cFeature::featuretype(string featurename) {
int npos = featurename.find(";");
if (npos >= 0) {
featurename = featurename.substr(0, npos);
}
string type(featuretypes[featurename]);
if (type.empty()) {
GErrorStr += featurename + "missing in featuretypes map.\n";
}
if (featurename == "__test_efel_assertion__") // for testing only
throw EfelAssertionError("Test efel assertion is successfully triggered.");
string type = featuretypes[featurename];
if (type != "int" && type != "double")
throw std::runtime_error("Unknown feature name: " + featurename);
return type;
}

Expand Down
48 changes: 30 additions & 18 deletions efel/cppcore/cppcore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,36 +108,48 @@ static void PyList_from_vectorstring(vector<string> input, PyObject* output) {
}

static PyObject*
_getfeature(PyObject* self, PyObject* args, const string &type) {
_getfeature(PyObject* self, PyObject* args, const string &input_type) {
char* feature_name;
PyObject* py_values;

int return_value;
if (!PyArg_ParseTuple(args, "sO!", &feature_name, &PyList_Type, &py_values)) {
PyErr_SetString(PyExc_TypeError, "Unexpected argument type provided.");
return NULL;
}

string feature_type = pFeature->featuretype(string(feature_name));

if (!type.empty() && feature_type != type){
PyErr_SetString(PyExc_TypeError, "Feature type does not match");
try {
string feature_type = pFeature->featuretype(string(feature_name));

if (!input_type.empty() && feature_type != input_type){ // when types do not match
PyErr_SetString(PyExc_TypeError, "Feature type does not match");
return NULL;
}

if (feature_type == "int") {
vector<int> values;
return_value = pFeature->getFeature<int>(string(feature_name), values);
PyList_from_vectorint(values, py_values);
return Py_BuildValue("i", return_value);
} else { // double
vector<double> values;
return_value = pFeature->getFeature<double>(string(feature_name), values);
PyList_from_vectordouble(values, py_values);
return Py_BuildValue("i", return_value);
ilkilic marked this conversation as resolved.
Show resolved Hide resolved
}
}
catch(EfelAssertionError& e) { // more specialised exception
PyErr_SetString(PyExc_AssertionError, e.what());
return NULL;
}

if (feature_type == "int") {
vector<int> values;
return_value = pFeature->getFeature<int>(string(feature_name), values);
PyList_from_vectorint(values, py_values);
} else if (feature_type == "double") {
vector<double> values;
return_value = pFeature->getFeature<double>(string(feature_name), values);
PyList_from_vectordouble(values, py_values);
} else {
PyErr_SetString(PyExc_TypeError, "Unknown feature name");
catch(const std::runtime_error& e) {
PyErr_SetString(PyExc_RuntimeError, e.what());
return NULL;
}
catch(...) {
PyErr_SetString(PyExc_RuntimeError, "Unknown error");
return NULL;
}

return Py_BuildValue("i", return_value);
}


Expand Down
2 changes: 1 addition & 1 deletion tests/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def test_nonexisting_feature():
trace['stim_end'] = [75]

pytest.raises(
TypeError,
RuntimeError,
efel.getFeatureValues,
[trace],
['nonexisting_feature'])
Expand Down
14 changes: 13 additions & 1 deletion tests/test_cppcore.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def test_getFeature_failure(self): # pylint: disable=R0201
return_value = efel.cppcore.getFeature("AP_amplitude", feature_values)
assert return_value == -1

@pytest.mark.xfail(raises=TypeError)
@pytest.mark.xfail(raises=RuntimeError)
def test_getFeature_non_existant(self): # pylint: disable=R0201
"""cppcore: Testing failure exit code in getFeature"""
import efel
Expand Down Expand Up @@ -209,3 +209,15 @@ def test_caching(self, feature_name):
assert contents.count(f"Calculated feature {feature_name}") == 1
# make sure Reusing computed value of text occurs twice
assert contents.count(f"Reusing computed value of {feature_name}") == 2


def test_efel_assertion_error():
"""Testing if C++ assertion error is propagated to Python correctly."""
import efel
efel.reset()
trace = {
"stim_start": [25],
"stim_end": [75],
}
with pytest.raises(AssertionError):
efel.getFeatureValues([trace], ["__test_efel_assertion__"])
Loading