[JOSS] replace thinkdsp with librosa.core.tone

[faroit]

the included thinkdsp.py file is only used for a single function - CosSignal. I would suggest to copy just this function (and add unit tests for this) or replace this with librosa, a very popular audio framework, that includes a tone generator as well: https://librosa.org/librosa/generated/librosa.core.tone.html

This issue is part of a JOSS submission review

[lockepatton]

Hi @faroit,

So sorry - this has been on the back burner, but I'm ready to make this a priority these next weeks.

I looked into the package you referenced, and found it couldn't do some of the minor odd specifics I needed. Switching to librosa would require more of a major rehaul of the code that feels necessary given our scope.

Looking into thinkdsp.py, there are some major class dependencies that the code uses in order to run the simple Sinusoid object, but I've pulled together some basic tests that have now been committed in ./tests/test_thinkdsp.py.

For your reference, I've commented out the portions of thinkdsp.py that are not strictly necessary. While the tests don't cover 100% of the processes, I think my tests cover the basics. Let me know if this is enough to fix this issue.

Cheers, Locke

[faroit]

I looked into the package you referenced, and found it couldn't do some of the minor odd specifics I needed. Switching to librosa would require more of a major rehaul of the code that feels necessary given our scope.

Curious about the specifics that you need. I am sure @bmcfee would love to hear about them and you could think of adding this to librosa via PR, which would allow more users to get access to these functionalities.

If this is too specific, I am okay with removing the unnecessary code form thinkdsp.py

[lockepatton]

Hi @faroit - thanks for the reply. Looking back over this, I think removing the unnecessary code is the best way to go. I've already commented the unnecessary code out; let me know if you would rather I delete it entirely or if leaving it commented (for possible future updates) is okay. At this point, let me know if you need anything else from me to complete revisions for this submission. Thanks!

[lockepatton]

Planning on closing this issue, but feel free to reopen if I misread your comment @faroit.

[faroit]

I think removing the unnecessary code is the best way to go. I've already commented the unnecessary code out; let me know if you would rather I delete it entirely or if leaving it commented (for possible future updates) is okay. At this point, let me know if you need anything else from me to complete revisions for this submission. Thanks!

i just had a quick look and I still think that there is a lot of code in thinkdsp.py that is not used from within sonipy, I would recommend to strip down everything except the parts that are useful for running CosSignal