using units in channels.tsv in read_raw_bids ?

[agramfort]

playing with ds003775 I see that units say the data in the edf file are in uV but it's read by read_raw_bids as V.

openneuro-py download --dataset=ds003775 --include=sub-001

import mne
from mne_bids import BIDSPath, read_raw_bids

bp = BIDSPath(
    root='./ds003775',
    subject='001',
    task='resteyesc',
    datatype='eeg',
    session='t1',
    suffix='eeg',
    extension='.edf',
)

raw = read_raw_bids(bp)
# raw = mne.io.read_raw(bp)
raw.load_data()
# raw.apply_function(lambda x: x * 1e-6)
raw.plot()

I saw the pb with: https://output.circle-artifacts.com/output/job/869cb730-d785-46aa-9c16-afc668bf6b70/artifacts/0/site/examples/ds003775/sub-010_ses-t1_task-resteyesc_report.html from mne-tools/mne-bids-pipeline#585

@sappelhoff @hoechenberger any idea? can I consider the edf files broken? as mne.io.read_raw_edf screws up also the units

🙏

[sappelhoff]

is it just that the BIDS metadata are wrong and it's actually in V?

[agramfort]

even if I change the channels.tsv file to have V and not uV it does not change anything

…

Message ID: ***@***.***>

[hoechenberger]

even if I change the channels.tsv file to have V and not uV it does not
change anything

Message ID: @.***>

This bit seems like a bug in MNE-BIDS then...

[agramfort]

digging into the reader the .edf file does not contain units. So it defaults to 1 for scaling and so it's off by 1e6. Now I don't know what to do with the channels.tsv as if the units in the tsv copy the unit in the edf file then we could apply the scaling twice.

Here is an attempt #1045

it drives me nuts to have file format where units can be omitted...

[sappelhoff]

If units can be omitted in EDF maybe we need to add a units param to mne.io.read_raw_edf and then in mne-bids pass an argument to that param, where we previously read the arg from the BIDS sidecar?

[agramfort]

interesting idea. what would be the logic? if the units are not present in the edf files we require to pass it on read_raw_edf? if we pass it it overwrites the units in the file?

…

Message ID: ***@***.***>

[sappelhoff]

Yes to both of your questions (IMHO) ... default should be None. and if we overwrite, we also warn.

[hoechenberger]

@agramfort

if the units are not present in the edf files we require to pass it on read_raw_edf?

I'd say yes; fail hard and force users to specify the unit.

if we pass it it overwrites the units in the file?

If the param is present and the units are specified in the file, we should either fail, or at least print a big fat warning…

Are there actually situations where users might want to override the units? I'm worried that if we allow this override, we're about to open a can of worms, as this will creep into the reader functions for other file formats too over time. I'd rather force users to fix their input files than us allowing this override...

But then again, we actually quite often see users running into scaling issues and I don't think we currently have a "clean" way to address those, right? So maybe we should add this feature for all readers? 🤔

[agramfort]

not all readers have this issue and the format is clear ;) it's really an EDF problem. to proceed step by step let's add a units parameter that is None by default and for now can be not None if units are actually missing from the file. And I would do units : str or list of str for now.

…

Message ID: ***@***.***>

[sappelhoff]

@agramfort can this be closed now?

[hoechenberger]

Closing as addressed upstream in mne-tools/mne-python#11099; actually reading the units from channels.tsv is to be addressed in #1045

[hoechenberger]

I just realized that the title of this issue is exactly what we want to solve in #1045, so I'm re-opening.