mne-python icon indicating copy to clipboard operation
mne-python copied to clipboard

Add support for dictionary-type `ref_channels` in `set_eeg_reference()`

Open qian-chu opened this issue 1 year ago • 5 comments

Implement #12283

To do list:

  • [x] Change docdict["ref_channels_set_eeg_reference"] in mne/utils/docs.py to include dict
  • [x] Change set_eeg_reference() to implement dict based re-referencing
    • [x] Modify _check_before_reference or create new _check_before_dict_reference
    • [x] Modify _apply_reference() or create new _apply_dict_reference()

qian-chu avatar Jan 16 '24 19:01 qian-chu

@AlexLepauvre I've come up with a short to-do list of things we may need to change to complete the PR. Feel free to modify it! My initial commit only changed the doc and slightly changed reference. Everything is subject to change as you see fit :)

qian-chu avatar Jan 16 '24 20:01 qian-chu

Do you think we need a formal test for the addition (in test_reference)?

qian-chu avatar May 03 '24 15:05 qian-chu

Code, tests, and docstring all updated and we are ready for review!

qian-chu avatar Jun 07 '24 08:06 qian-chu

I've pushed a few commits to tidy up the implementation, and added some inline explanations below in a self-review (the commit messages should hopefully also be helpful in understanding what I did).

I didn't yet look closely at the adequacy of the tests, other than to ensure they all pass locally for me, and adapt them slightly to account for the change making the second return value be None (see comments below for reasoning).

One thing that I think is still missing is handling of SSP projectors, which may become invalid after referencing. The existing _check_before_reference helper has code to handle this; that should be refactored into another helper func and called from the new _check_before_dict_reference func too.

Just pushed a fix for the SSP projectors. Do you think we should also add a test for this in test_set_eeg_reference_dict?

qian-chu avatar Jul 29 '24 19:07 qian-chu

@drammock When you have time would you mind re-reviewing? Thanks!

qian-chu avatar Aug 15 '24 15:08 qian-chu