search

mne-tools / mne-connectivity

Connectivity algorithms that leverage the MNE-Python API.
https://mne.tools/mne-connectivity/dev/index.html
BSD 3-Clause "New" or "Revised" License
68 stars 34 forks source link

sampling frequency defaults to 2*pi in spectral_connectivity and phase_slope_index #9

Closed adam2392 closed 1 year ago

adam2392 commented 3 years ago

To my knowledge, there is no reason for the sfreq to default to 2 * np.pi inside these methods. It seems like a parameter that should be passed in from the user via the Raw.info object.

larsoner commented 3 years ago

We have to have some default in case ndarray is passed. Looking at scipy.signal, though, it seems like 2 * np.pi is less standard than just 2 (which yields freqs normalized between 0 and 1). Let's change the default to None which means "use sfreq if available, and must be provided if it's not". If it's provided and the data is an instance with info, then it should be checked for equivalence and an error raised if not the same. WDYT?

britta-wstnr commented 3 years ago

As far as I can see, sfreq is mostly used to "verify" data size and that the frequency to be estimated is okay (in terms of Nyquist etc)., see e.g. _prepare_connectivity. So I wonder if that default actually worked?

adam2392 commented 1 year ago

We have to have some default in case ndarray is passed. Looking at scipy.signal, though, it seems like 2 * np.pi is less standard than just 2 (which yields freqs normalized between 0 and 1). Let's change the default to None which means "use sfreq if available, and must be provided if it's not". If it's provided and the data is an instance with info, then it should be checked for equivalence and an error raised if not the same. WDYT?

As far as I can see, sfreq is mostly used to "verify" data size and that the frequency to be estimated is okay (in terms of Nyquist etc)., see e.g. _prepare_connectivity. So I wonder if that default actually worked?

Coming back to this, and I think IIUC, I will make a short fix to the API and requires sfreq to be passed in if numpy array.

adam2392 commented 1 year ago

I'll do this before v0.5 #118