Move PyPredict custom exception definition to predict.c#6
Conversation
…failures in C code as well.
There was a problem hiding this comment.
It seems a little weird to call this exception class "PredictException" in Python but "Exception" in C. Would it make more sense to use PredictException everywhere? That way it'll be clear what your PyErr_SetString calls are doing.
There was a problem hiding this comment.
I'm trying to remember why I did it this way - I had them as PredictException's before and changed it. Line length issues ?:-/
It would be prettier to have them as predict.Exception, but I get the feeling that's frowned upon.
Anyway, fixing.
|
Okay, other than the comments above (which are all minor), this looks good to me. |
+ Rename 'Exception' to more desriptive 'PredictException' + Fixed reference count bug on PyObject * transit + Use likely more descriptive python error messages in List append failure + Handle case where PythonifyObservation fails to build value
|
Comments addressed. Could you take a quick look at the quick_predict function and make sure there aren't any lurking bugs with my use of a local goto? |
|
Cool, this looks good to me. (In theory I think you're also supposed to free your new py_obs reference if the subsequent PyList_Append fails, but I can't imagine that ever being a problem in practice -- it's a tiny memory leak, and it'll only happen if you're already out of memory for other reasons. Manual memory management sucks.) |
Move PyPredict custom exception definition to predict.c
2 participants
Also added exceptions on failure cases in C code.
@mconst-nanosatisfi