Closed hoechenberger closed 3 years ago
Sounds like a good feature to support, PR welcome :)
I'd be willing to give it a short, but how can we test whether the files we create are actually BV-compatible? I don't have BV analyzer to work with…
I have a license for BV analyzer at one of the institute computers, so I could test some files in the near future.
Overall though I think it's more important that fieldtrip eeglab mne brainstorm etc can read it (given that we comply with the BV spec).
coming here from my comment https://github.com/bids-standard/pybv/pull/75#discussion_r655194870 which was about:
implicitly declaring everything as referenced to a "common reference", if I'm not mistaken
I think this is poor wording in the BV spec. Based on my experience, the "common reference" does not imply a "common average reference", but rather a single reference channel that is used for all other channels. for Brain Products systems this is often FCz.
See my linked comment above where I post contents from a file that support this interpretation.
EDIT: This could also be clarified with an Email to Brain Products support (support@brainproducts.com) --> in the past I received quick replies (1-3 days in most cases) that were helpful.
I sent an email, click details to see it:
We got a reply :tada: and maybe another one will follow by the techsupport.
It looks like "it depends" on both the acquisition hardware (will need more information) & Software (BV recorder vs pycorder).
Currently it looks to me like we should say that an empty field means "all channels referenced to a common channel that is not further specified". WDYT @hoechenberger ?
Do you have an old pycorder file that contains info in the "comment" section that could be of interest to us?
I sent a follow-up inquiry, so that we can proceed with implementing this feature.
We got two more answers. Based on them, I think we should proceed as follows:
reference_channel=None
(the default), pybv's old behavior is retained: The docstring will say that this corresponds to assuming a reference channel that is common to all channels, but which is not further specified.reference_channel=<some channel name that is in the data>
, we error if the channel is not all zeros ... else we accept itreference_channel=<some channel name that is **NOT** in the data>
, we accept it. In future extensions we might think about extending our API so that such cases can be documented in the [Comment]
section - but for now I feel that this is overkill.None
- we simply don't allow it. Otherwise, the rules from above applyFirst additional answer:
Second additional answer:
Sounds good! We might also consider adding an option to add a zero-channel if the specified reference channel is not in the data (by default this should be False
of course).
I'm against adding channels automatically, if the user wants them, they should add them manually. They already need to pass the data as a numpy array, meaning they will have to know their way around numpy anyway... Actually, it's quite common that the ref channel is NOT included in the recording, so I'm not convinced that we should make the "non-standard" practice so easy 😅
Converting data from a proprietary format to BV with @dnacombo, we found ourselves in need of a way to specify the reference channel(s) used. It seems that currently,
pybv
doesn't support adding this information, implicitly declaring everything as referenced to a "common reference", if I'm not mistaken:There should be a way to specify reference channels.