Apply hilbert stc by BabaSanfour · Pull Request #12323 · mne-tools/mne-python

BabaSanfour · 2023-12-24T07:00:56Z

Fixes #12310.

Updating mne.filter.MixinFilter methods to work with Source Estimated data

…ilbert to work on SourceEstimate object

for more information, see https://pre-commit.ci

BabaSanfour · 2023-12-24T07:05:43Z

working to fix the failed tests

for more information, see https://pre-commit.ci

BabaSanfour · 2023-12-25T03:34:26Z

All tests passed.
I think the functions are decent.
For SourceEstimate.resample the function overwrite the MixinFilter.resample so I did not update the latter. However I think that SourceEstimate.resample can be integrated into MixinFilter.resample. Not sure if this would have any implication on speed but I think it would improve the code.

larsoner

Looks like a good start! See some inline comments.

Any new functionality should be accompanied by unit tests, so in mne/tests/test_source_estimate.py we'll need some basic tests of the new functions that have been added for at least the SourceEstimate class.

doc/changes/devel/12323.newfeature.rst

mne/filter.py

mne/source_estimate.py

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

for more information, see https://pre-commit.ci

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

for more information, see https://pre-commit.ci

BabaSanfour · 2024-02-05T18:47:17Z

I have addressed all of the suggestions. I also added one function to test to stc.apply_function in test_source_estimate.py . For the object.apply_hilbert function, the test_evoked.py has the test function for raw, epochs and evoked objects, I wonder if the tests should be added in the same function. Also, to test object.apply_function there is pre-existing data in ../io/tests/data . Should we use for example raw data to generate sources? or is there a way to get access for stc_test_data ?

larsoner

For the object.apply_hilbert function, the test_evoked.py has the test function for raw, epochs and evoked objects, I wonder if the tests should be added in the same function.

Sure!

Should we use for example raw data to generate sources? or is there a way to get access for stc_test_data ?

I would just use np.random.default_rng(0).normal(...) like here:

mne-python/mne/tests/test_source_estimate.py

Lines 373 to 379 in e6b49ea

    
           def _fake_stc(n_time=10, is_complex=False): 
        
               np.random.seed(7) 
        
               verts = [np.arange(10), np.arange(90)] 
        
               data = np.random.rand(100, n_time) 
        
               if is_complex: 
        
                   data.astype(complex) 
        
               return SourceEstimate(data, verts, 0, 1e-1, "foo")

But don't use np.random.seed, that's bad practice -- use class-based methods after instantiating a default_rng instead (modern best practice for NumPy random that we haven't fully adopted yet but should at some point!).

mne/filter.py

mne/utils/docs.py

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

for more information, see https://pre-commit.ci

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

for more information, see https://pre-commit.ci

larsoner

Just three minor tweaks, marking for merge-when-green, thanks in advance @BabaSanfour !

mne/tests/test_source_estimate.py

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

BabaSanfour added 2 commits December 24, 2023 01:41

adapt Filter.Mixin, FilterMixin.savgol_filter and FilterMixin.apply_h…

63a4bbb

…ilbert to work on SourceEstimate object

description

5193165

BabaSanfour requested review from agramfort, dengemann, drammock and larsoner as code owners December 24, 2023 07:00

pre-commit-ci bot and others added 2 commits December 24, 2023 07:01

[pre-commit.ci] auto fixes from pre-commit.com hooks

821cfae

for more information, see https://pre-commit.ci

fix PR id

8653f67

BabaSanfour and others added 3 commits December 24, 2023 21:20

fix return to line

982ceb5

return to line/description

712de2b

[pre-commit.ci] auto fixes from pre-commit.com hooks

d7ddfa3

for more information, see https://pre-commit.ci

larsoner reviewed Jan 2, 2024

View reviewed changes

BabaSanfour and others added 12 commits February 4, 2024 16:55

update doc

6bddbd3

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

drop saving attributes

e20756a

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

change vertice to vertex

5fe9183

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

fix doctoring

2c316a3

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

update check_preload to account for stc

535f095

use _picks_to_idx

992d366

add _picks_to_idx to ensure picks is an ndarray of int

7eebf01

[pre-commit.ci] auto fixes from pre-commit.com hooks

6cbdb28

for more information, see https://pre-commit.ci

Update mne/source_estimate.py

45290a8

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

operate on VectorSourceEstimates

11365d7

add test stc.apply_function()

7444a65

[pre-commit.ci] auto fixes from pre-commit.com hooks

a41e3a1

for more information, see https://pre-commit.ci

BabaSanfour requested a review from larsoner February 5, 2024 21:43

larsoner reviewed Feb 6, 2024

View reviewed changes

mne/filter.py Show resolved Hide resolved

mne/filter.py Outdated Show resolved Hide resolved

mne/utils/docs.py Outdated Show resolved Hide resolved

fix docstring

d27c619

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

pre-commit-ci bot and others added 5 commits February 6, 2024 17:30

[pre-commit.ci] auto fixes from pre-commit.com hooks

3bb892c

for more information, see https://pre-commit.ci

Update docstring mne-tools#2

94cb0ed

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

Update documentation

1168051

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

add stc.apply_hilbert test

7f61515

[pre-commit.ci] auto fixes from pre-commit.com hooks

fb3d53d

for more information, see https://pre-commit.ci

BabaSanfour requested a review from larsoner February 6, 2024 20:41

larsoner approved these changes Feb 8, 2024

View reviewed changes

mne/tests/test_source_estimate.py Outdated Show resolved Hide resolved

mne/tests/test_source_estimate.py Outdated Show resolved Hide resolved

mne/tests/test_source_estimate.py Outdated Show resolved Hide resolved

Apply suggestions from code review

56dd3b8

larsoner enabled auto-merge (squash) February 8, 2024 17:56

Merge branch 'main' into apply_hilbert_stc

2b2f865

larsoner merged commit 7045590 into mne-tools:main Feb 13, 2024

snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024

Apply hilbert stc (mne-tools#12323)

495fe26

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

	def _fake_stc(n_time=10, is_complex=False):
	np.random.seed(7)
	verts = [np.arange(10), np.arange(90)]
	data = np.random.rand(100, n_time)
	if is_complex:
	data.astype(complex)
	return SourceEstimate(data, verts, 0, 1e-1, "foo")

Uh oh!

Conversation

BabaSanfour commented Dec 24, 2023

Uh oh!

BabaSanfour commented Dec 24, 2023

Uh oh!

BabaSanfour commented Dec 25, 2023

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BabaSanfour commented Feb 5, 2024

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants