Add EGI as manufacturer

[anandsaini024]

Added EGI as a manufacturer in the eeg_manufacturers dict for extension '.bin' and '.mff'. This PR closes issue #1005.

[welcome]

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

[adam2392]

Can you add a change log entry and a unit test for writing?

[adam2392]

Hey just checking in to see if you hit any snags.

I suspect if you add necessary dataset to here it should test the writing of this EGI data format:

mne-bids/mne_bids/tests/test_write.py

Lines 103 to 107 in b4250ec

	test_eegieeg_data = [
	('EDF', 'test_reduced.edf', _read_raw_edf),
	('Persyst', 'sub-pt1_ses-02_task-monitor_acq-ecog_run-01_clip2.lay', _read_raw_persyst), # noqa
	('NihonKohden', 'MB0400FU.EEG', _read_raw_nihon)
	]

Not entirely sure tho. The tests definitely need a bit of organizing :p.

[anandsaini024]

y just checking in to see if you hit any snags.

I suspect if you add necessary dataset to here it

Hey, was busy with something else in between. I will look at this tomorrow.

[hoechenberger]

@anandsaini024 It appears you'll have to amend allowed_extensions_meg in config.py

[mscheltienne]

It's still going to fail because with this test file: '/Users/scheltie/mne_data/MNE-testing-data/EGI/test_egi.mff'
when doing: events, _ = mne.events_from_annotations(raw, event_id=None)
events is returned as an empty array array([], shape=(0, 3), dtype=int64)
while after conversion (write_raw_bids) and re-reading with raw2 = read_raw_bids(bids_path=bids_output_path)
the events returned with events2, _ = mne.events_from_annotations(raw2)
do have a single element.. array([[ 0, 0, 99999]]).

[hoechenberger]

Oh my… why is this being so complicated? 🤯

[scott-huberty]

Hi all, is this something that still needs work/help with? MNE-BIDS support for EGI files is something that has been on my radar, and @drammock this is a task I was going to propose for the code sprint.

[adam2392]

Yes I think it will involve some refactoring of the unit tests

[mscheltienne]

If you have time to finish this PR during the code sprint next week, it would be super appreciated! I can give you a hand if you want. For now, the issue I ran into 3 weeks ago is related to the conversion to the brain vision format. Take the code snippet below:

from mne.datasets import testing
from mne.io import read_raw_brainvision, read_raw_egi
from mne_bids import BIDSPath, write_raw_bids


directory = testing.data_path() / "EGI"
raw = read_raw_egi(directory / "test_egi.mff")

root="C:/Users/Mathieu/Downloads/tests/bids1"
bids_path = BIDSPath(
    subject="01", session="01", run="01", acquisition="01",
    task="testing", root=root)
write_raw_bids(raw, bids_path)
raw2 = read_raw_brainvision(bids_path.fpath)

The raw.annotations is empty while the loaded raw2.annotations contains one segment. Up to the call to pybv.write_brainvision, the raw.annotations is empty. So it's either the writer from pybv or the reader from mne that is responsible for this segment. I am not very familiar with the brainvision format, maybe @sappelhoff or @cbrnr you know where this segment is coming from?

[scott-huberty]

Hey @mscheltienne , thanks! It would be great to briefly meet with you this week if you have some time, just so I can catch myself up and get a hold of the work that's been done already.

[sappelhoff]

looks like a reasonable suggestion to me! Though maybe it warrants its own issue and accompanying PR.

[scott-huberty]

looks like a reasonable suggestion to me! Though maybe it warrants its own issue and accompanying PR.

Sounds good! I will open an issue and begin working on a PR.

[codecov]

Codecov Report

Merging #1006 (57833f7) into main (b90dde8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1006   +/-   ##
=======================================
  Coverage   95.19%   95.19%           
=======================================
  Files          25       25           
  Lines        3787     3787           
=======================================
  Hits         3605     3605           
  Misses        182      182           

Impacted Files	Coverage Δ
mne_bids/config.py	97.61% <ø> (ø)
mne_bids/write.py	97.14% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us.

[mscheltienne]

@hoechenberger @sappelhoff @adam2392 Alright, this PR is (finally) green!
I had to add a version check for MNE 1.1 in the tests following mne-tools/mne-python#10851
Have a look and merge if happy :)

Thanks @scott-huberty for working on this one!

[mscheltienne]

And by the way, test_eegieeg was a pain to work with. IMO, It's way too long and testing too many aspects in the same function.

[sappelhoff]

And by the way, test_eegieeg was a pain to work with. IMO, It's way too long and testing too many aspects in the same function.

Yes 😞

• Reduce code duplication in tests, test functions in correct test modules #212
• [maint] Possible refactor/organization of unit tests for readability and decreased time (if possible) #678

[sappelhoff]

@anandsaini024 can you please leave a comment on #1028 with:

• your full name
• your ORCID (if you have one)
• your affiliation (if you have one)

[hoechenberger]

formatting change for improved readability

[welcome]

🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪