-
Notifications
You must be signed in to change notification settings - Fork 111
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
Why are "physio.tsv.gz" and its JSON currently not valid for eeg/meg/ieeg ? #990
Comments
Agreed. I would move these out of the |
hmm, interesting consideration. In preparing the BEPs for MEG, EEG and iEEG we have not discussed adding the physio and stim files. Although for iEEG there was a discussion on electrical stimulation, but that was non-continuous and ended up as https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/04-intracranial-electroencephalography.html#electrical-stimulation Starting from the MEG side: these systems usually have a lot of AUX channels (that are sampled synchronously and stored in the main MEG dataset) and we decided to keep them in there when present. Hence the The Storing auxiliary data in a TSV file next to the main data (given the modality in the directory name) is a strategy that makes sense to me. However, eye tracking is not physiology; I would rather see it as behaviour. In this example there was physio (EMG, PPG and breathing) from a single brainamp and eyetracking from a SMI system. In that case I used the modality labels In the PET BEP (almost final) there is a There might be a general pattern appearing here, which would be that the main data can be complemented with aux data in a relatively minimal It would be good if you guys could have a look at the PET BEP and specifically on the synchronization between the _blood.tsv and the other (in this case PET) data. Also for a |
Thanks Robert! I generally agree with you and the things you say in your last paragraph. For some things you say however, I have a slightly different take - let me address them point by point:
I don't have a strong opinion about what type of data eyetracking is, as long as I can clearly see that it's eyetracking, which I think is quite clear from:
And currently, BIDS does support eyetracking in
I think that physio+stim do not need to be explicitly added --> in my understanding they are already as much part of BIDS as This is also why I see my reported issue as a bug --> I see no reason why physio+stim files should be fine in a
I would also prefer to go with BEP020, but using
I agree --> and my intention is not to blow up |
#1001 expands physio and stim files to eeg ieg meeg. The specification in 06-physiological-and-other-continuous-recordings.html is very explicit about func, so that should either be updated to include *egs or made ambiguous enough to cover them. There are some key/value regex that only appear in functional file names (rec, dir, ce...) or in the *eg in general (split) or in ieeg (space). Would these keys ever appear in physio or stim file names? |
are you referring to the fact that the examples have These two examples are because when merging MEG,EEG,iEEG into BIDS, we never made the effort to adjust the Physio section in BIDS, so it is still pretending that BIDS is only about MRI :-) I am trying to address this here: bids-standard/bids-specification#513
as far as I can see from the spec, physio or stim files would contain every entity that the file contains the refer to ... PLUS an optional see this part in the physio section:
|
could |
reading the behavioral sction makes me think so 🤔 what do you think? |
I don't think it is neither explicitly allowed nor explicitly forbidden. For https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/07-behavioral-experiments.html it at least says "Behavioral experiments (with no MRI)" and there is a dedicated Imagine me recording PPG and breathing during a behavioural experiment. All elements by themselves could be stored in BIDS, but in this specific situation it is not clear from the specification how. I would do it like this:
|
Another question: is it required that these I think it makes sense to make that consistent, i.e. make it optional for all tsv files to be gzipped (or not). |
I think we should make it more future proof and already anticipate upcoming modalities like PET. So rather than extending the includelist from [ |
yes, I would do it like that as well. We could improve coverage of the example in the physio section. But probably after these two are merged:
good point and it has recently been raised by @yarikoptic in bids-standard/bids-specification#472 That issue could use some more opinions to converge on a solution.
I agree cc @rwblair ... if we don't find an elegant fix for this, it'd just mean adding 4 lines to each of the "regexp blocks" (stim.tsv, stim.json, physio.tsv, physio.json)
|
…ities allow phsysio and stim tsvs and jsons in *eg directories fixes #990
while going though
_physio
and_stim
files (see bids-standard/bids-specification#513), I realized that the validator currently does not accept files likesub-05_task-matchingpennies_recording-eyetracking_physio.tsv.gz
:This is because the regular expressions for:
_physio.tsv.gz
_stim.tsv.gz
_physio.json
_stim.json
are specified for
func
, but not foreeg
,meg
, andieeg
.https://github.com/bids-standard/bids-validator/blob/6f123f091b0951fe3cfcc670fbbb27f7fd1fb449/bids-validator/bids_validator/rules/file_level_rules.json#L119-L145
I think this should be changed, and it is valid BIDS to have a
sub-05_task-matchingpennies_recording-eyetracking_physio.tsv.gz
in an EEG dataset at the nesting specified in my example above.@robertoostenveld, what is your opinion? Have you stumbled over this issue before?
The text was updated successfully, but these errors were encountered: