Improved error handling #16
Conversation
Rafagd
commented
Oct 14, 2020
Rafagd
commented
- pypesq now has an on_error parameter that decides the behaviour of the function on error
- cypesq will now call cypesq_retvals and raise an exception if the returned value is negative
- cypesq_retvals will not crash-and-burn if no utterances are detected in the buffer
- pesq_measure will now return early and override the Error_Flag with its own values instead of exposing internal Error_Flags
- pow_of exit(1)'s were changed to asserts
- Adding vIM buffer files to .gitignore
- cypesq will now call cypesq_retvals and raise an exception if the returned value is negative - cypesq_retvals will not crash-and-burn if no utterances are detected in the buffer - pesq_measure will now return early and override the Error_Flag with its own values instead of exposing internal Error_Flags - pow_of exit(1)'s were changed to asserts - Adding vIM buffer files to .gitignore
|
thanks for your work. could you add correspoding testing functions for the new error throwing feature inside the folder |
|
Does the library support multi threaded usage? If so, I think it would be better to at least use thread locals for error reporting so that error reporting is thread safe as well. |
|
I once tried to release the GIL in PESQ and it turned out, that PESQ uses some global statements (It either broke or returned wrong values, I don't remember). Maybe that would be a point for the future: Release the GIL and make the code thread safe. |
|
Is there a reason, why you changed so many code lines? |
It was the only way to reliably return the errors using the Error_Flag that was already present instead of having 2 different error returning mechanisms like in the other PR. Previously there was no way I could know what values would be returned, as each function inside the pesq_measure does a different thing with the Error_Flag and pesq_measure was just returning the same value to the outside world. That said, I think I'll double check for leaks. Ps.: Most of the lines are reindentation as well. |
|
Then I probably don't see what you changed. and this changed the indent, so the github diff is difficult to read. |
|
So, the way the function was written, it checks for the Error_Flag then it calls the next function that may change the flag. To make sure I'm always talking about the last Error_Flag, I do the thing first, then I check the flag and return early. Otherwise I would need to double check the flag multiple times. I may need to add a goto there to do some clean-up on failure. |
There is an option to hide all white space changes. |
TIL. OMG, that is so much better to read! |
There is no need for it.
|
hi @Rafagd, thanks for your contribution. |
|
Sorry for the delay, just did it now. |
