[FIX] add optional _acq-<acquisition> to meg, eeg, ieeg, channels

[gpiantoni]

Following up on the discussion in Neurostars, acq-<acquisition is an optional field for meg, eeg, ieeg, channels according to the Entity Table. This fix updates the individual pages related to meg, eeg, and ieeg.

[sappelhoff]

indeed, and #610 would fix that as well. But as #610 will probably be merged only in some time, I think merging this PR ahead of #610 is sensible.

A new change is to add acq as optional for CHANNELS files. That makes sense to me, because if some data is recorded with a different ACQ, it may affect things such as reference, sampling frequency, etc, which can be written into CHANNELS.

@gpiantoni could you please add an optional ACQ to eeg and meg CHANNELS templates as well? As far as I can see you only did that for iEEG currently.

[gpiantoni]

Thank you for the review. Sorry, I added the additional field to eeg and meg channels as well.

[sappelhoff]

cc @bids-standard/raw-electrophys - please review

[MaxvandenBoom]

Looks ok Gio

[effigies]

Can you add acq: optional to the channels schema?

bids-specification/src/schema/auxdatatypes/channels.yaml

Lines 11 to 15 in 744b3d4

	entities:
	sub: required
	ses: optional
	task: required
	run: optional

Then run python tools/bids_schema.py.

[tsalo]

Thank you for this change. Two notes- first, the options for the value in an entity key-value pair are "index" and "label". The appropriate one for acq is label. Second, once you've updated the schema file, please use bids_schema.py to update the entity table, rather than editing the entity table file directly.

[tsalo]

The entity table shouldn't be edited directly. It should be generated from the bids_schema.py script.

[gpiantoni]

OK, sorry about that. I restored the original 99-entity-table.md.

[gpiantoni]

Sorry about editing the entity table. Unfortunately, I get an error when running python3 tools/bids_schema.py:

Traceback (most recent call last):
File "tools/bids_schema.py", line 466, in
_main()
File "tools/bids_schema.py", line 461, in _main
args.pop('func')
KeyError: 'func'

Unfortunately I don't have time into debugging this issue.

[sappelhoff]

No worries @gpiantoni we will take it from here. Thanks for starting on the PR.

[sappelhoff]

PS: @gpiantoni if you want to add your name to the BIDS contributors, you are welcome to do so. Just edit this Wiki page: https://github.com/bids-standard/bids-specification/wiki/Recent-Contributors

we'll make sure your name is added to the appendix before the next release of BIDS.

[arnodelorme]

I did not see @robertoostenveld comment on this. I am personally agnostic to the addition of this optional file to the EEG, MEG, iEEG BIDS format. For EEG though, there is a JSON file with already a lot of information about data acquisition.

[robertoostenveld]

Hi Arno, this was discussed (also by me) on https://neurostars.org/t/two-amplifiers-for-ieeg-recordings/17492. The PR follows up on that discussion.

Rather than only adding <acq>-label to the text of the spec (via the technical definition), I think that all optional entities in https://bids-specification.readthedocs.io/en/stable/99-appendices/04-entity-table.html have to be considered. Either we add all of them everywhere (also for MRI), or we have to document in another way that there are additional optional entities that are not in the example.

[robertoostenveld]

Can you add acq: optional to the channels schema?

bids-specification/src/schema/auxdatatypes/channels.yaml

Lines 11 to 15 in 744b3d4

	entities:
	sub: required
	ses: optional
	task: required
	run: optional

Then run python tools/bids_schema.py.

This is not only for channels.tsv, but also for the eeg data and json file itself.

UPDATE, oh this is already in https://github.com/bids-standard/bids-specification/blob/master/src/schema/datatypes/eeg.yaml and the discussion actually should better continue in the PR #677