Something that's been on my mind. Probably a 1.0 matter.
The problem
MNE-Python has a nice seperation of functions that operate on arrays, and those that operate on MNE objects, especially for spectral analyses. MNE-Connectivity doesn't. E.g., spectral_connectivity_epochs can accept array-like time series data, array-like source estimates, Epochs, and EpochsSpectrum/TFR objects.
It makes the code that bit messier and harder to maintain. Also for users, it leads to a very busy docstring (which I've been told from beginners to MNE is a bit tricky to understand):
Proposed solution
I would like there to be more consistency with MNE-Python, e.g., a spectral_connectivity_epochs function that accepts MNE objects, and a spectral_connectivity_epochs_array function that accepts arrays. I'd also propose this for non-spectral connectivivty analysis functions, but the spectral connectivity stuff is definitely the messiest right now.
Yes, there would be significant changes needed to existing code, but that's why I'd consider it a 1.0 matter.
Proposal to drop support for time series data from spectral connectivity functions
However, the question is what the accepted data should represent, especially for the spectral functions: do they expect spectral coefficients to be passed, or would time series data still be allowed?
Dropping support for time series data would allow us to clean up the code a lot. Would make maintenance much easier as it's really quite complicated to understand in it's current state. It also adds very little burden to users: if they have Epochs data, just pass in the result of Epochs.compute_psd/tfr(outputs="complex"); and likewise the results of the psd_array_... and tfr_array_... functions for array data.
However, there is the question of how source estimate data is handled if support for time series data is dropped. I've never worked with this before, so I don't know what the equivalent pipeline looks like using MNE-Python spectral functions.
Open to discussion on this, but I'd argue in the long-term it's a big improvement to code maintainability and understandability of the spectral connectivity functions (not accepting loads of different forms of data and the caveats of what other params need to be passed for each).
Something that's been on my mind. Probably a 1.0 matter.
The problem
MNE-Python has a nice seperation of functions that operate on arrays, and those that operate on MNE objects, especially for spectral analyses. MNE-Connectivity doesn't. E.g.,
spectral_connectivity_epochscan accept array-like time series data, array-like source estimates,Epochs, andEpochsSpectrum/TFRobjects.It makes the code that bit messier and harder to maintain. Also for users, it leads to a very busy docstring (which I've been told from beginners to MNE is a bit tricky to understand):
Proposed solution
I would like there to be more consistency with MNE-Python, e.g., a
spectral_connectivity_epochsfunction that accepts MNE objects, and aspectral_connectivity_epochs_arrayfunction that accepts arrays. I'd also propose this for non-spectral connectivivty analysis functions, but the spectral connectivity stuff is definitely the messiest right now.Yes, there would be significant changes needed to existing code, but that's why I'd consider it a 1.0 matter.
Proposal to drop support for time series data from spectral connectivity functions
However, the question is what the accepted data should represent, especially for the spectral functions: do they expect spectral coefficients to be passed, or would time series data still be allowed?
Dropping support for time series data would allow us to clean up the code a lot. Would make maintenance much easier as it's really quite complicated to understand in it's current state. It also adds very little burden to users: if they have
Epochsdata, just pass in the result ofEpochs.compute_psd/tfr(outputs="complex"); and likewise the results of thepsd_array_...andtfr_array_...functions for array data.However, there is the question of how source estimate data is handled if support for time series data is dropped. I've never worked with this before, so I don't know what the equivalent pipeline looks like using MNE-Python spectral functions.
Open to discussion on this, but I'd argue in the long-term it's a big improvement to code maintainability and understandability of the spectral connectivity functions (not accepting loads of different forms of data and the caveats of what other params need to be passed for each).