search

datalad-datasets / hcp-structural-preprocessed

Preprocessed structural MR data from the WU-Minn HCP1200 dataset. More info in README.
https://github.com/datalad-datasets/hcp-structural-preprocessed/blob/master/README.md
2 stars 1 forks source link

bids: not yet legitish BIDS #2

Open yarikoptic opened 4 years ago

yarikoptic commented 4 years ago

quick notes

mih commented 4 years ago

re participants.tsv: Do you know where basic demographics of these people are available?

re CHANGES: Not clear what you mean by "Might be worth tagging this bids dataset releases"

re _T1wDividedByT2w: desc approach sounds good, but .bidsignore might be just as suitable.

yarikoptic commented 4 years ago

re participants.tsv: Do you know where basic demographics of these people are available?

no - haven't looked

re CHANGES: Not clear what you mean by "Might be worth tagging this bids dataset releases"

Use git tag -a to tag "releases" of the datasets with reflection in CHANGES (part of BIDS)

re _T1wDividedByT2w: desc approach sounds good, but .bidsignore might be just as suitable.

suitable to pacify bids-validator, but not suitable e.g. to make this data usable out of the box by bids aware tools (such as pybids)

loj commented 4 years ago

missing participants.tsv. Probably that is why bids-validator immediately fails to recognize it as BIDS

My understanding of the bids spec is that the participants.tsv file is optional, and thus wouldn't be why the bids-validator fails.

re _T1wDividedByT2w: I will ask the BEP003 authors what they think (bids-standard/bids-specification#462). In the meantime, do we want to go with _desc-dividedByT2w_T1w?

yarikoptic commented 4 years ago

@loj re participants.tsv: oh hoh -- you must be right! in BIDS spec it is shmooshed together with phenotype/ which is optional for sure. And I got a victim of a not-yet-addressed https://github.com/bids-standard/bids-validator/issues/792 . To troubleshoot in this case I just left one subject (still trips), got all the data for it and it stopped "tripping" that generic validator error... heh heh

here is the output of the validator in that case ```shell $> bids-validator $PWD bids-validator@1.4.2 1: [ERR] Files with such naming scheme are not part of BIDS specification. This error is most commonly caused by typos in file names that make them not BIDS compatible. Please consult the specification and make sure your files are named correctly. If this is not a file naming issue (for example when including files not yet covered by the BIDS specification) you should include a ".bidsignore" file in your dataset (see https://github.com/bids-standard/bids-validator#bidsignore for details). Please note that derived (processed) data should be placed in /derivatives folder and source data (such as DICOMS or behavioural logs in proprietary formats) should be placed in the /sourcedata folder. (code: 1 - NOT_INCLUDED) ./DATA_USE_AGREEMENT.md Evidence: DATA_USE_AGREEMENT.md ./README.md Evidence: README.md ./sub-100206/anat/sub-100206_T1wDividedByT2w.nii.gz Evidence: sub-100206_T1wDividedByT2w.nii.gz ./sub-100206/anat/sub-100206_desc-dc-restore_T1w.nii.gz Evidence: sub-100206_desc-dc-restore_T1w.nii.gz ./sub-100206/anat/sub-100206_desc-dc-restore_T2w.nii.gz Evidence: sub-100206_desc-dc-restore_T2w.nii.gz ./sub-100206/anat/sub-100206_desc-dc_T1w.nii.gz Evidence: sub-100206_desc-dc_T1w.nii.gz ./sub-100206/anat/sub-100206_desc-dc_T2w.nii.gz Evidence: sub-100206_desc-dc_T2w.nii.gz Please visit https://neurostars.org/search?q=NOT_INCLUDED for existing conversations about this issue. 2: [ERR] Not a valid JSON file. (code: 27 - JSON_INVALID) ./dataset_description.json @ line: 6 character: 22 Evidence: "Version": "", Please visit https://neurostars.org/search?q=JSON_INVALID for existing conversations about this issue. 3: [ERR] The compulsory file /dataset_description.json is missing. See Section 03 (Modality agnostic files) of the BIDS specification. (code: 57 - DATASET_DESCRIPTION_JSON_MISSING) Please visit https://neurostars.org/search?q=DATASET_DESCRIPTION_JSON_MISSING for existing conversations about this issue. 4: [ERR] No BIDS compatible data found for at least one subject. (code: 67 - NO_VALID_DATA_FOUND_FOR_SUBJECT) ./sub-100206 Please visit https://neurostars.org/search?q=NO_VALID_DATA_FOUND_FOR_SUBJECT for existing conversations about this issue. 1: [WARN] The recommended file /README is missing. See Section 03 (Modality agnostic files) of the BIDS specification. (code: 101 - README_FILE_MISSING) Please visit https://neurostars.org/search?q=README_FILE_MISSING for existing conversations about this issue. Summary: Available Tasks: Available Modalities: 8 Files, 338.41MB 0 - Subjects 1 - Session If you have any questions, please post on https://neurostars.org/tags/bids. ```

Sure thing validator does not yet support _desc field from the BEP. But I noted that _desc-dc-restore should probably be redone to not have - in dc-restore since - is not yet allowed in the values for the keys... I kept suggesting adding it (https://github.com/bids-standard/bids-specification/issues/226) , but there is resistance and most likely + would be introduced (someone yet to do a PR proposing it) instead to be allowed in the values. So you might better use it instead here or chime in on that issue to possibly tilt the scales toward allowing regular '-'.

yarikoptic commented 4 years ago

as for demographics:

loj commented 4 years ago

But I noted that _desc-dc-restore should probably be redone to not have - in dc-restore since - is not yet allowed in the values for the keys...

@yarikoptic Thanks for pointing this out. :-) I saw that you suggested/volunteered to create a PR allowing for both the - and + options, so for now, I think it makes sense to leave in the -.

re _T1wDividedByT2w: T1wT2wratio seems to be the popular suggestion.

yarikoptic commented 4 years ago

re - -- given the speed other, more trivial changes, get accepted, I would have not hold breath awaiting for - to become legit. + is an easier candidate. And here I feel that dcrestore is still readable, and would immediately become "legit". just my 1c.