Adding dependencies and writing tests for audio.jl #13
Conversation
Manifest.toml
Outdated
| @@ -0,0 +1,258 @@ | |||
| # This file is machine-generated - editing it directly is not advised | |||
There was a problem hiding this comment.
Manifest.toml should never be committed: first remove it, then add it to .gitignore
|
Great, thanks for adding tests! However at the moment there are many failings: https://travis-ci.org/github/JuliaMusic/MusicProcessing.jl/jobs/725473247 Can you have a look? |
|
Yup, I wrote those tests for functions that I havent fixed yet. Those functions rely on TFR.jl which I'll most likely focus on next |
src/TFR.jl
Outdated
| @@ -18,7 +18,7 @@ function spectrogram(audio::SampleBuf{T, 2}, | |||
| hopsize::Int = windowsize >> 2; | |||
| window = hanning, kwargs...) where T | |||
| noverlap = windowsize - hopsize | |||
| (mapslices(data, 1) do data | |||
| (mapslices(audio.data, dims=1) do data | |||
| DSP.spectrogram(data, windowsize, noverlap; fs = audio.samplerate, window = window, kwargs...) | |||
| end)[:] | |||
There was a problem hiding this comment.
I've noticed here that instead of putting [:] at the end and making an unecessary copy, we can wrap the entire thing in a vec(...) function.
There was a problem hiding this comment.
I assume this applies for all instances of [:] ? If so I'll change them all I as I go
|
Thanks for continuing to work here @jparcill ! Little by little, this will be in soon! |
jparcill
force-pushed
the
jparcill_writing_tests
branch
from
43b486a to
167b941
Compare
|
Great news. Though I haven't written up the thorough tests yet, just trying the functions out in a jupyter notebook with a WAV file I had lying around, a lot of the functionality looks good! Including:
to name a few |
|
Very nice! Once you have some basic tests down, I'll do a final review and we merge this! |
| DSP = "717857b8-e6f2-59f4-9121-6e50c889abd2" | ||
| FFTW = "7a1cc6ca-52ef-59f5-83cd-3a7055c09341" | ||
| FixedPointNumbers = "53c48c17-4a7d-5ca2-90c5-79b7896eea93" | ||
| Requires = "ae029012-a4dd-5104-9daa-d747884805df" | ||
| SampledSignals = "bd7594eb-a658-542f-9e75-4c4d8908c167" | ||
| Unitful = "1986cc42-f94f-5a68-af5c-568840ba703d" |
There was a problem hiding this comment.
O, forgot to say this here. In the compat entry we need an entry for every package used, to allow for registration in the General registry (you can simply use the versions you tested this with in Jupyter)
|
@ashwani-rathee thanks for the interest. FOr sure @jparcill can guide you more, but the main thing I believe is to write passing tests in the |
|
@jparcill I have never written a test before,can you guide on what I should learn\read???How long this task of writting test will continue??I saw the files of chroma.jl/constantq.jl empty that could also be starting point for me,but I would love to try hands on testing.. |
|
@jparcill can you share your jupyter notebook in which you're exploring?? |
Hey @ashwani-rathee sure. How should I send it? |
|
Yeah that makes sense! School's unfortunately getting busy again and it'd be great to have more helping hands |
|
OKay, no problem, just make sure to underline in #10 what remains from this PR ! |
|
Functionality is pretty much there for audio.jl after writing tests and also playing around with them in a jupyter notebook. Functions that I haven't fixed yet are the ones that depend on other files like TFR.jl