Improve I/O with EEGLAB

[mscheltienne]

I have to work a bit with EEGLAB, and I'm not very familiar with it (yet) but I already noticed a couple of improvements that could be made to the I/O in my opinion.

Reader

Channel type is hard-coded to 'eeg' here while EEGLAB seems to support a larger variety of channel types.
From the documentation:

‘EEG’, ‘MEG’, ‘EMG’, ‘ECG’, ‘Events’, etc.

Proposition: Try to retrieve the channel types saved by EEGLAB and convert it to one of MNE channel types (e.g. 'EEG' -> 'eeg', 'Events' -> 'stim').

Writer / Export

Looking at export_set in eeglabio, it has an argument ref_channels that is not used by the export code. If I'm not mistaken, the referencing information is present in Info, so it could be passed to the .set file via this argument.

eeglabio.raw.export_set(
        fname, data=raw.get_data(picks=ch_names), sfreq=raw.info['sfreq'],
        ch_names=ch_names, ch_locs=cart_coords, annotations=annotations)

Support for channel type is not included in eeglabio, I would like to add it jackz314/eeglabio#4
Once added to eeglabio, the exporter could make use of this additional argument.

---

TODO:

• Add support for MNE channel types in the reader (MRG: Add support to retrieve channel type from ['chanlocs']['type'] with EEGLAB reader #9990)
• Add a warning when exporting with unapplied projectors + warning in documentation (MRG: Issue warning when exporting with unapplied projectors and improve docstring #9994)
• Add support to export MNE channel types (depends on jackz314/eeglabio#4)

[cbrnr]

Sounds good! However, MNE does not keep track of the reference channel so unfortunately this won't work automatically.

[mscheltienne]

@cbrnr Arf! Too bad for that, but to be honest my main priority is the channel types :)

Now that I think about it, what about projectors that are not yet applied? I think, for now, the export is not applying them.. maybe we could add a proj argument also to add the possibility to apply them prior to export.
If so, we could also check for the presence of a common average reference projector, in which case we could set ref_channels='average' during the export. If MNE is not keeping track of the reference channel(s), then I think this is the only case in which ref_channels can be define.

[cbrnr]

True, but I'd rather leave it to the user to define what they want to export. That way, we don't have to add another parameter (which as you correctly say only works for that single case).

[mscheltienne]

But projectors may contain other types of projectors that the user may want to set. I think the method .apply_proj() is not very well known because of the argument proj present in many MNE functions. At the very least, I think the export should not silently get rid of the projectors without any warning for the user in the docstring / notes section.

[cbrnr]

EEGLAB is mostly used for EEG analysis, and setting an average reference in MNE does not use a projector by default. The other popular projector in MNE is SSP, which is used in MEG analysis. I think if someone uses one of these two projectors in EEG analysis (or some other custom projector), they probably know what they are doing and should be able to apply_proj() before exporting. But yes, feel free to mention this explicitly in the docs (and also issue a warning if there are unapplied projectors).

[agramfort]

Sounds good! However, MNE does not keep track of the reference channel so unfortunately this won't work automatically.

what do you mean?

[mscheltienne]

I think he meant that if you reference to a specific channel, e.g. CPz, this information is lost and all you know is that a custom reference has been applied, but not which one.

[agramfort]

@CBNR what was the reason for this? we have no fif constant tag to store this? note that the info['chs'][0]['loc'][3:6] should contain the location of the reference for EEG channel of index 0 but this is still not done properly. I was not aware of this FIF requirement when this was written years ago...

…

[cbrnr]

what was the reason for this? we have no fif constant tag to store this?

Yes. And my opinion is still that we should move beyond FIFF when it is holding us back. EEG data is not stored in FIFF very often, and I believe other formats such as EDF or BrainVision do support storing the reference name.

[mscheltienne]

@cbrnr @agramfort Related to this, mne.io.proj._needs_eeg_average_ref_proj is not doing what it's supposed to.

# ANT Neuro dataset referenced to 'CPz' by the amplifier
raw= mne.io.read_raw_fif(fname,  preload=True)

_needs_eeg_average_ref_proj(raw.info)
Out: True

raw.set_eeg_reference(ref_channels=['M1', 'M2'])
_needs_eeg_average_ref_proj(raw.info)
Out: False

[hoechenberger]

@agramfort @cbrnr

what was the reason for this? we have no fif constant tag to store this?

Yes. And my opinion is still that we should move beyond FIFF when it is holding us back.

We discussed this before, and yes, I too believe this is a massive shortcoming of MNE that we urgently need to resolve for EEG users.

Here's the discussion thread, in which we discussed some concrete ideas how to best address this, all within the FIFF specs if I'm not mistaken: #8962

[larsoner]

I would phrase it as "we should extend the FIF spec when it's holding us back." So far the times we've done it I think it has forced us to really think about and distill what's necessary about the new information. Without this constraint (e.g., if we adopted a looser approach for example based on HDF5) I imagine things would have been messier long term.

[larsoner]

... and I think #8962 is good example actually, even though we haven't settled on a solution there

[hoechenberger]

... and I think #8962 is good example actually, even though we haven't settled on a solution there

I'd like to second that

[agramfort]

can we discuss this tomorrow during the MNE office hours?

…