Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: fix manufacturer checking in _channels_tsv() #1101

Closed
dengemann opened this issue Nov 18, 2022 · 6 comments · Fixed by #1102
Closed

ENH: fix manufacturer checking in _channels_tsv() #1101

dengemann opened this issue Nov 18, 2022 · 6 comments · Fixed by #1102
Assignees
Labels

Comments

@dengemann
Copy link
Member

dengemann commented Nov 18, 2022

Description of the problem

in write_bids_raw, allow_preload=True should allow writing EEG files from unsupported manufacturers / files formats (e.g. EGI .raw).
Yet, _channels_tsv() makes strong assumptions about extensions e.g. when looking up ignored channels.
Could it make sense to implement IGNOREd_CHANNELS as a default dict or only look that up if the extension is known?

Steps to reproduce

import tempfile
import mne
from mne_bids import write_raw_bids, BIDSPath

fname = mne.datasets.testing.data_path() / 'montage' / 'egi_dig_test.raw'

raw = mne.io.read_raw_egi(fname)

with tempfile.TemporaryDirectory() as tmpdir:
    bp = BIDSPath(subject='1', root=tmpdir, datatype='eeg', task='foo')

    write_raw_bids(raw, bp, format='BrainVision', allow_preload=True)

Expected results

converting to BrainVision from EGI .raw (once preloaded)

Actual results

channels_tsv(raw, fname, overwrite)
    113 # get the manufacturer from the file in the Raw object
    114 _, ext = _parse_ext(raw.filenames[0])
--> 115 manufacturer = MANUFACTURERS[ext]
    117 ignored_channels = IGNORED_CHANNELS.get(manufacturer, list())
    119 status, ch_type, description = list(), list(), list()

KeyError: '.raw'

Additional information

Platform: Linux
Python: 3.8.13 | packaged by conda-forge | (default, Mar 25 2022, 06:04:10) [GCC 10.3.0]
mne: 1.2.0
numpy: 1.23.4 {unknown linalg bindings (threadpoolctl module not found: No module named 'threadpoolctl')}
scipy: 1.9.1

@dengemann dengemann added the bug label Nov 18, 2022
@dengemann
Copy link
Member Author

I can actually add a reproducible example based on MNE testing files, will do in a bit

@hoechenberger
Copy link
Member

We need to convert the format, can you try with passing the format parameter to write_raw_bids()?

@dengemann
Copy link
Member Author

@hoechenberger added reproducible example on top

@hoechenberger
Copy link
Member

hoechenberger commented Nov 18, 2022

Thanks!

Yeah we should clearly skip this check if the data has already been preloaded. That would be the solution to the entire problem, right?

@dengemann
Copy link
Member Author

That's also my understanding.

@hoechenberger
Copy link
Member

@dengemann In your example, you forgot to actually preload the data. Here's the "corrected" MWE (which still produces the same error):

import tempfile
import mne
from mne_bids import write_raw_bids, BIDSPath

fname = mne.datasets.testing.data_path() / 'montage' / 'egi_dig_test.raw'

raw = mne.io.read_raw_egi(fname, preload=True)

with tempfile.TemporaryDirectory() as tmpdir:
    bp = BIDSPath(
        subject='1', task='foo', suffix='eeg', extension='.vhdr',
        datatype='eeg', root=tmpdir
    )
    write_raw_bids(raw, bp, format='BrainVision', allow_preload=True)

Produces

Writing '/var/folders/2k/nkw3dkx97ssdv08xmdgfnjkc0000gn/T/tmprycdflf0/dataset_description.json'...
Writing '/var/folders/2k/nkw3dkx97ssdv08xmdgfnjkc0000gn/T/tmprycdflf0/sub-1/eeg/sub-1_task-foo_eeg.json'...
Traceback (most recent call last):
  File "/private/tmp/mwe-bids.py", line 15, in <module>
    write_raw_bids(raw, bp, format='BrainVision', allow_preload=True)
  File "<decorator-gen-620>", line 12, in write_raw_bids
  File "/Users/hoechenberger/Development/mne-bids/mne_bids/write.py", line 1861, in write_raw_bids
    _channels_tsv(raw, channels_path.fpath, overwrite)
  File "/Users/hoechenberger/Development/mne-bids/mne_bids/write.py", line 115, in _channels_tsv
    manufacturer = MANUFACTURERS[ext]
KeyError: '.raw'

We can deduce 2 things here:

  1. The error message is not helpful at all
  2. If the data has been preloaded, the original format shouldn't matter, but currently does

@hoechenberger hoechenberger self-assigned this Nov 18, 2022
hoechenberger added a commit to hoechenberger/mne-bids that referenced this issue Nov 18, 2022
agramfort pushed a commit that referenced this issue Nov 20, 2022
…ormat conversion (#1102)

* Fix writing preloaded data of a non-BIDS format with automated format conversion

Fixes #1101

* Simpler

* Better error message

* Add test

* Better error message

* Add changelog entry

* Fix error message

* Add requires_testing_data decorator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants