mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.72k stars 1.32k forks source link

apply_hilbert for source estimated data #12310

Closed BabaSanfour closed 9 months ago

BabaSanfour commented 10 months ago

Describe the new feature or enhancement

Currently apply_hilbert can be applied only to raw/epochs/epoched data through, for example, epochs. apply_hilbert. The idea is to change this whole function into the time_frequency module. Then the epochs/raw/evoked/source data can be an argument to the function (EX: mne.time_frequency.EpochsTFR)

Describe your proposed implementation

It can be a new class in the tfr.py file. It would be based on the ancient .apply_hilbert function.

Describe possible alternatives

The alternative that is being used now is script hilbert function for source data or apply_hilbert for other types. The proposed implementation will make the function independent of the data type/object

Additional context

I am happy to propose an implementation. I just want to check with maintainers for directions before starting. As this is a huge change in a lot of classes/files. I think it should be discussed before starting the implementation.

welcome[bot] commented 10 months ago

Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴

larsoner commented 10 months ago

Alternatively we could make FilterMixin work for SourceEstimate instances. I think in principle it should be doable -- would need to make FilterMixin.filter, FilterMixin.savgol_filter, and FilterMixin.resample (which already exists for SourceEstimate classes) work properly. WDYT?

BabaSanfour commented 10 months ago

Yes that would work too; By "already exists for SourceEstimate classes" you refer to the resample function only right? could not find the other implementations.

larsoner commented 10 months ago

Yeah just resample exists currently

BabaSanfour commented 10 months ago

ok thanks! I will take that into account when making the changes.

BabaSanfour commented 10 months ago

@larsoner the function has been added