BUG: "magic number" in vocalpy.spectral.sat should be n_fft arg

[ralphpeterson]

I am attempting to compute acoustic features in vocalpy on some sample data and have run into an issue using the vocalpy.spectral.sat function.

>>> import numpy as np
>>> from vocalpy.spectral import sat
>>> from vocalpy import Audio

>>> data = np.load('../some_audio.npy')

>>> audio = Audio(data, 125000)

>>> sat(audio, n_fft=512, hop_length=256)

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[24], [line 1]
----> [1] sat(audio, n_fft=512, hop_length=256)

File [~\Anaconda3\envs\vocalpy\lib\site-packages\vocalpy\spectral\sat.py:98], in sat(audio, n_fft, hop_length, freq_range)
        [96] windows = librosa.util.frame(audio_pad, frame_length=n_fft, hop_length=hop_length, axis=0)
        [97] tapers = scipy.signal.windows.dpss(400, 1.5, Kmax=2)
---->   [98] windows1 = windows * tapers[0, :]
        [99] windows2 = windows * tapers[1, :]
        [101] spectra1 = np.fft.fft(windows1, n=n_fft)

ValueError: operands could not be broadcast together with shapes (138,512) (400,) 

tapers and windows shapes aren't compatible. How are the params being selected for scipy.signal.windows.dpss here, @NickleDave? It looks like the first argument is window size in the time domain, so maybe 400 is hard coded, as it is the default value of n_fft in the sat function? I confirmed that the function runs locally after changing 400 to n_fft. Not positive what the other arguments are doing. Happy to fix this if it's a bug, or even happier to be told that this is a simple user error.

[NickleDave]

😳 This is definitely a bug! Thank you for catching it @ralphpeterson.

Yes, can you fix that hard-coded 400 in line 97 so that it's n_fft instead?

vocalpy/src/vocalpy/spectral/sat.py

Line 97 in f2a339d

tapers = scipy.signal.windows.dpss(400, 1.5, Kmax=2)

We should also add a unit test that varies this parameter. I can add that in the PR if you make the fix.

[NickleDave]

@all-contributors please add @ralphpeterson for bug

[allcontributors]

@NickleDave

I've put up a pull request to add @ralphpeterson! 🎉

[ralphpeterson]

yep, will fix and submit a pull request

[NickleDave]

🙏 thank you

Just icymi I'm fixing a couple other things in #108 so you'll want to sync your fork.

Very very minor detail but if you don't mind giving your branch a semantically meaningful name like fix-n_fft-arg-fixes-#100, that just helps keep track of what's what.

Really appreciating your help!

edit: nevermind I lied -- I'm going to hold off on bumping the minimum version of Python until numba supports Python 3.12
#108 (comment)

Once we get your bugfix in I'll release a new patch version