bids-standard / bids-validator

Validator for the Brain Imaging Data Structure
https://bids-standard.github.io/bids-validator/
MIT License
184 stars 109 forks source link

headshape and electrode placement files cause error #564

Open monkeyman192 opened 6 years ago

monkeyman192 commented 6 years ago

In the BIDS specification the electrode placement and headshape files (if they exist) are expected to have the file type the originally has (eg. for the KIT data I have, .elp and .hsp respectively) However, the validator expects the extension to be .pos (cf. here and here) I think this can be fixed easily by simply allowing the extension to be anything (including no extension as I believe is the case for BTi data...)

sappelhoff commented 6 years ago

+1, this needs to be fixed.

robertoostenveld commented 6 years ago

elp is not a native KIT format, right? It sounds like a BESA format with spherical coordinates that is also used by EEGLAB. BIDS for raw MEG should only include those files generated by the native equipment and recording software. Converted files should go in "derivatives", template files from other software (such as EEGLAB) should not go with the recorded raw data. Can you confirm the file format that is generated by the KIT software itself?

PS Note that you can always add extra files and use .bidsignore to ignore them in the validator.

monkeyman192 commented 6 years ago

The issue is not the fact that it is an .elp file per se. The issue is that the bids specification states the headshape and electrode placement files should be in a certain location with the original file type. However doing this raises an issue as the validator expects the file type to always be .pos But in answer to your question I believe the .elp and .hsp files are unprocessed files straight from the device that obtains them (but I would need to confirm with someone from work who actually does this process and would know)

robertoostenveld commented 6 years ago

I can for sure imagine there being more files than just .pos, but only the ones that are part of the original recording should be allowed by the validator. I was just expressing my doubts about the .elp format.

On 17 Sep 2018, at 15:07, monkeyman192 notifications@github.com wrote:

The issue is not the fact that it is an .elp file per se. The issue is that the bids specification states the headshape and electrode placement files should be in a certain location with the original file type. However doing this raises an issue as the validator expects the file type to always be .pos But in answer to your question I believe the .elp and .hsp files are unprocessed files straight from the device that obtains them (but I would need to confirm with someone from work who actually does this process and would know)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/INCF/bids-validator/issues/564#issuecomment-422007989, or mute the thread https://github.com/notifications/unsubscribe-auth/AA2346oA7adBOYQewhJtSl4Lis-sS04hks5ub56SgaJpZM4WopNO.

monkeyman192 commented 6 years ago

For the test data we have with MNE-BIDS, the KIT data has both headshape and electrode placement files with the .txt file type, and the BTi data has no file type. (Although looking at the sample data in MNE it has both .hsp/.elp and .txt versions of the data.)

sappelhoff commented 6 years ago

@jasmainak @robertoostenveld the MEG BIDS specification says that headshape files should look like this: *_headshape.<manufacturer_specific_format>

Yet all we see in the validator is the .pos manufacturer_specific_format.

see here:

https://github.com/INCF/bids-validator/blob/b1574fdedde1610e8acba12f514dbe267942df84/bids_validator/rules/session_level_rules.json#L60

and here:

https://github.com/INCF/bids-validator/blob/b6a52fb8fe846ec9d2dfec8267e52cbfd22824bf/bids_validator/rules/file_level_rules.json#L201

is .pos the most common one? Have the other extensions been "forgotten"? I have never worked with headshape files, so some input would be appreciated in order to solve this issue, which is tied to multiple other issues.

jasmainak commented 6 years ago

@sappelhoff I think this should be fixed. However, I am not sure allowing all extensions is a good idea. At the time I wrote the regular expressions, I saw the example and therefore included .pos. There is a list here which we could probably use?

sappelhoff commented 6 years ago

However, I am not sure allowing all extensions is a good idea

Shouldn't we include all those extensions that are being used? Otherwise perfectly fine files will trigger a validation error.

There is a list here which we could probably use?

Interestingly, that list does not include .pos 🤔

monkeyman192 commented 6 years ago

.hsp is also not on that list. I agree that including all possible values isn't a great solution, but do we even know every possible extension that can be used? I think unless we can actually decide that there is a specific list of extensions that the files will always have we should just allow any extension (especially since the BIDS specification doesn't place any restrictions on the file type...)

jasmainak commented 6 years ago

okay, why not :) who is making the PR? :)

sappelhoff commented 6 years ago

@jasmainak see #585