Closed marcsello closed 4 years ago
What you say makes perfect sense, to be honest, I dont remember how this signal handler was kept in audioTrainTest, i guess it should be from some very old version that included realtime audio recording. Anyway, you are absolutely true and thank you for the comment. I am testing the PR and approving in a while
Importing this library overrides (or at least tries) the default signal handler for the UNIX signal SIGINT. This is not just a general bad practice but breaks compatibility with various frameworks, and causes the writing of complex applications that uses this library very hard.
A few examples where this behavior of the library do cause problems:
ValueError: signal only works in main thread
KeyboardInterrupt
breaks as no such exceptions would be raised. Instead sys.exit(0) is called which causes an unclean exit. This behavior requires special hacks to work around (using__exit__
method of objects, etc.) which are not nice nor reliable.These issues are caused by the first few lines of audioTrainTest.py: https://github.com/tyiannak/pyAudioAnalysis/blob/32abdd748d9a26f1fcf5ee58860c0eb7b45df1ec/pyAudioAnalysis/audioTrainTest.py#L22-L27
I believe a library should never attempt tho handle signals. This is entirely the responsibility of the main application. Thus that signal handler should be removed at least from the beginning of the file.
If this functionality is required when individual functions run at command line, then I suggest to only override the signal, when this code is run manually at the command line (e.g.: using
__name__
). So that it won't break code it's imported in.Soon I'll post a pull request that I think fixes this issue.