MRG: [DOC] MNE does no longer auto apply average ref

[sappelhoff]

closes #7471

These are the files I could catch that did still assume MNE-Python to auto apply an average ref projection. Also, the docstr of set_eeg_reference was still assuming that.

I wasn't sure with tutorials/preprocessing/plot_55_setting_eeg_reference.py --> see my inline "todo" comment there marked with XXX

PS: What was the PR that removed the auto average ref behavior of MNE?

[sappelhoff]

note: I am uncertain about how to change the lines above. Will MNE-Python or will it NOT apply auto average refs?

[drammock]

As far as I can tell, it is not automatically applied:

• _check_reference is called in _apply_inverse here
• _check_reference (here) calls _needs_eeg_average_ref_proj (here), which returns True if (1) there are EEG channels present, and (2) a custom ref has not been applied, and (3) EEG average reference proj is not already there.

Instead, the user will get a ValueError.

In fact, it doesn't even seem to be possible any more to avoid this constraint by setting average reference to an empty list:

raw = mne.io.read_raw_fif(sample_data_raw_file, verbose=False).crop(tmax=30).load_data()               
raw = raw.set_eeg_reference([])                                                                        
mne.minimum_norm.inverse._check_reference(raw)
# ValueError: EEG average reference is mandatory for inverse modeling, use set_eeg_reference method.

[sappelhoff]

Thanks @drammock

[codecov]

Codecov Report

Merging #7474 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #7474   +/-   ##
=======================================
  Coverage   90.08%   90.08%           
=======================================
  Files         454      454           
  Lines       82539    82608   +69     
  Branches    13051    13066   +15     
=======================================
+ Hits        74353    74420   +67     
- Misses       5360     5361    +1     
- Partials     2826     2827    +1     

[sappelhoff]

mne-python/mne/channels/channels.py

Line 281 in 25f6e5e

def set_eeg_reference(self, ref_channels='average', projection=False,

class SetChannelsMixin(MontageMixin) has a set_eeg_reference method which is just pointing to mne.set_eeg_reference.

Instead of duplicating the docstring in that class method, can we somehow "import it" from the original mne.set_eeg_reference?

[drammock]

Instead of duplicating the docstring in that class method, can we somehow "import it" from the original mne.set_eeg_reference?

You can take a look at mne/utils/docs.py to see docdict; frequently re-used docstring elements are put there, and then inserted via %(docdict_key_name)s where they're needed. There are also decorators @copy_function_doc_to_method_doc and @copy_doc.

[sappelhoff]

Cool, this decorator would be perfect for it:

mne-python/mne/utils/docs.py

Line 900 in 25f6e5e

def copy_function_doc_to_method_doc(source):

except for two things:

1. the method I would like to receive a copy of the function docstring does not have a parameter that the function has (copy, the 3rd param of the function)
2. the function has two return values, the method only one (that one is congruent between method and function though).

perhaps the decorator could be extended with an optional parameter to delete a list of params by indices (currently, it's only deleting the first param, index 0) ... and extend the param deletion to a deletion of return values as well?

[larsoner]

perhaps the decorator could be extended with an optional parameter to delete a list of params by indices (currently, it's only deleting the first param, index 0) ... and extend the param deletion to a deletion of return values as well?

Maybe but it's not too difficult just to split the docstring into needed parts. Also we have to be wary of adding complexity to our docstring modifiers since they are executed on import.

See how I dealt with this sort of thing recently in MaxFilter:

https://github.com/mne-tools/mne-python/blob/master/mne/preprocessing/maxwell.py#L63-L93

There is a function that uses all the same params except for a couple (e.g., st_duration, st_correlation which are not allowed) so I grouped params and made entries in docs.py:

https://github.com/mne-tools/mne-python/blob/master/mne/utils/docs.py#L298-L384

[sappelhoff]

okay, thanks. Ready from my side.

[drammock]

@sappelhoff you have a "double line break" error in the docstring test. to iterate more quickly to find a solution, you can do make docstring locally from the root of the repo, to see if your fixes work.

[sappelhoff]

@sappelhoff you have a "double line break" error in the docstring test. to iterate more quickly to find a solution, you can do make docstring locally from the root of the repo, to see if your fixes work.

I tried several things now but couldn't find the issue :-/

[agramfort]

ok !

…

[drammock]

@sappelhoff I fixed the double line break error (locally, at least). The problem was that there was a line break between the end of the Returns section and the %(docdict_key)s code, and the thing that gets filled in by the %(docdict_key)s code begins with another linebreak. I also pushed a fix for slightly nicer formatting in the notes section.

[sappelhoff]

Thanks a lot @drammock

[larsoner]

Thanks @sappelhoff