[MRG] Added epilepsy example dataset #133

ftadel · 2018-12-03T03:30:41Z

No description provided.

sappelhoff

Thanks!

There are still some errors to be corrected though (see comments).

regarding the travis error

3: [ERR] The column names of the channels file must begin with ['name', 'type', 'units', 'sampling_frequency', 'low_cutoff', 'high_cutoff', 'notch', 'reference'] (code: 72 - MISSING_TSV_COLUMN_IEEG_CHANNELS)

you can coordinate with Chris H. see: https://github.com/bids-standard/bids-validator/pull/652

Regarding the error

4: [ERR] Empty files not allowed. (code: 99 - EMPTY_FILE)

I have raised this issue here: https://github.com/bids-standard/bids-validator/issues/651#issuecomment-442821513

and it needs to be discussed, cc @chrisfilo

sappelhoff · 2018-12-03T07:44:52Z

ieeg_epilepsy/sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-01_channels.tsv

+fz	EEG	uV	n/a	n/a	512	unknown		good
+cz	EEG	uV	n/a	n/a	512	unknown		good
+ecg1	ECG	uV	n/a	n/a	512	unknown		good
+ecg2	ECG	uV	n/a	n/a	512	unknown		good


you are missing a column for the last 5 rows, if it's not known, fill it with n/a

(see also below for similar)

sappelhoff · 2018-12-03T07:45:10Z

ieeg_epilepsy/sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-01_ieeg.json

+    "TaskName": "seizure",
+    "SamplingFrequency": 512,
+    "PowerLineFrequency": 50,
+    "SoftwareFilters", "n/a",


typo: comma should be a colon

(see also below for similar)

ftadel · 2018-12-03T12:24:13Z

Regarding the remaining warnings: should I add participants.json and _channels.json files?
Where can I find templates for these files? I couldn't find info about these in the specs or the article.

Regarding the empty files error: Were not supposed to post only empty data files in theis bids_examples repo?

sappelhoff · 2018-12-03T12:32:03Z

Regarding the remaining warnings: should I add participants.json and _channels.json files?
Where can I find templates for these files? I couldn't find info about these in the specs or the article.

Yes, please specify a participants.json which contains a description of all "non-custom keys" that you are using in the corresponding participants.tsv.

The "custom keys" are usually explicitly referred to (and described) in the specification ... such that additional information is not necessary. In your case sex throws an error, because there is no explanation about it in the specification, so you have to provide it. For more information on what's custom, you can see this file: https://github.com/bids-standard/bids-validator/blob/master/bids_validator/tsv/non_custom_columns.json

Regarding channels.json and electrodes.json, ... if you are using only the fields as defined in the specification, then you shouldn't add anything --> you are just getting an error, because the validator lags a bit behind the specification, and we have not yet added the "new custom columns" to the bids-validator. (This is a ToDo for us, cc @choldgraf )

Regarding a "template", please see: https://github.com/bids-standard/bids-examples/blob/bep006_eeg/eeg_matchingpennies/participants.json

ftadel · 2018-12-04T00:23:23Z

Thanks - Fixed
Is there anything else I should do with this dataset?

So if I understand well, there is no way I can post this iEEG dataset openneuro yet?

sappelhoff

great, thanks! It's looking good now.

RE: openneuro:

you would upload the full data there, so the "Empty File Error" wouldn't be applicable, would it?
I think openneuro accepts bids data as long as it does not throw errors ... the complaint about non-described columns in channels.tv and electrodes.tsv is just a warning (soon to disappear, once the validator catches up) ... so you should be fine uploading to openneuro.

If @choldgraf or @dorahermes agree, I can merge this.

ftadel · 2018-12-05T02:26:06Z

I think openneuro accepts bids data as long as it does not throw errors ... the complaint about non-described columns in channels.tv and electrodes.tsv is just a warning (soon to disappear, once the validator catches up) ... so you should be fine uploading to openneuro.

No errors because of the empty files or the columns in the .tsv files, but none of the .json ieeg files are accepted by the openneuro validator...

sappelhoff · 2018-12-05T05:43:31Z

ahhhh, I completely forgot ... openneuro is running bids-validator ... but for now, to validate iEEG data, we need to run bids-validator --bep010.

This will only change once iEEG is merged into the main spec. Actually, we could already make that step with the validator IMHO, what's your take on this @chrisfilo? Would it be okay to remove the optional EEG and iEEG flags of the validator now? Or is that something we should do as the "last step of the merging process"?

chrisgorgo · 2018-12-05T05:46:40Z

I would prefer to wait until it's merged into the spec. Hopefully won't be long now.

…

On Wed, Dec 5, 2018, 12:43 AM Stefan Appelhoff ***@***.*** wrote: ahhhh, I completely forgot ... openneuro is running bids-validator ... but for now, to validate iEEG data, we need to run bids-validator --bep010 . This will only change once iEEG is merged into the main spec. Actually, we could already make that step with the validator IMHO, what's your take on this @chrisfilo <https://github.com/chrisfilo>? Would it be okay to remove the optional EEG and iEEG flags of the validator now? Or is that something we should do as the "last step of the merging process"? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#133 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOkpx99gh_-9LtNKq-DDh5jel1KP56Aks5u110EgaJpZM4Y90Ra> .

sappelhoff · 2018-12-05T05:49:52Z

I would prefer to wait until it's merged into the spec. Hopefully won't be long now.

Mmh okay fair enough, that puts us into a dilemma situation though: @ftadel's data is part of iEEG bids, which we want to publish before merging ... but in order to publish, it needs to be uploaded (his choice: openneuro), which is only possible once the merge has happened ;-)

Solutions I see:

storing the data on another platform ... either temporarily or permanently
not sharing the full data (yet) ... but waiting for after the merge --> which might be before the actual print of the iEEG publication, so only the people reading the preprint would be confused that the real example data is not accessible.

…of contact length (len=2mm)

ftadel · 2019-02-11T16:00:43Z

When will this be merged?

sappelhoff · 2019-02-11T16:14:50Z

We are waiting for an approving review from @choldgraf or @dorahermes

It seems that in the meantime, adjustments to the validator also made this PR not pass:

Validating dataset ieeg_epilepsy/
	1: [ERR] Invalid JSON file. The file is not formatted according the schema. (code: 55 - JSON_SCHEMA_VALIDATION_ERROR)
		./sub-01/ses-post/ieeg/sub-01_ses-post_coordsystem.json
			Evidence:  should NOT have additional properties
	2: [ERR] Empty files not allowed. (code: 99 - EMPTY_FILE)
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-01_ieeg.eeg
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-01_ieeg.vhdr
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-01_ieeg.vmrk
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-02_ieeg.eeg
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-02_ieeg.vhdr
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-02_ieeg.vmrk
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-03_ieeg.eeg
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-03_ieeg.vhdr
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-03_ieeg.vmrk
	1: [WARN] Tabular file contains custom columns not described in a data dictionary (code: 82 - CUSTOM_COLUMN_WITHOUT_DESCRIPTION)
		./sub-01/ses-post/ieeg/sub-01_ses-post_space-MNI152_electrodes.tsv
			Evidence: Columns: name, x, y, z, size, hemisphere, group, type, manufacturer not defined, please define in: /space-MNI152_electrodes.json, /electrodes.json,/sub-01/sub-01_space-MNI152_electrodes.json,/sub-01/sub-01_electrodes.json,/sub-01/ses-post/sub-01_ses-post_space-MNI152_electrodes.json,/sub-01/ses-post/sub-01_ses-post_electrodes.json,/sub-01/ses-post/ieeg/sub-01_ses-post_space-MNI152_electrodes.json,/sub-01/ses-post/ieeg/sub-01_ses-post_electrodes.json
		./sub-01/ses-post/ieeg/sub-01_ses-post_space-orig_electrodes.tsv
			Evidence: Columns: name, x, y, z, size, hemisphere, group, type, manufacturer not defined, please define in: /space-orig_electrodes.json, /electrodes.json,/sub-01/sub-01_space-orig_electrodes.json,/sub-01/sub-01_electrodes.json,/sub-01/ses-post/sub-01_ses-post_space-orig_electrodes.json,/sub-01/ses-post/sub-01_ses-post_electrodes.json,/sub-01/ses-post/ieeg/sub-01_ses-post_space-orig_electrodes.json,/sub-01/ses-post/ieeg/sub-01_ses-post_electrodes.json
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-01_channels.tsv
			Evidence: Columns: reference, group not defined, please define in: /sub-01/ses-post/ieeg/sub-01_ses-post_channels.json, /sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_channels.json,/sub-01/ses-post/sub-01_ses-post_channels.json,/sub-01/ses-post/sub-01_ses-post_task-seizure_channels.json,/sub-01/sub-01_channels.json,/sub-01/sub-01_task-seizure_channels.json,/channels.json,/task-seizure_channels.json,/sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-01_channels.json
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-02_channels.tsv
			Evidence: Columns: reference, group not defined, please define in: /sub-01/ses-post/ieeg/sub-01_ses-post_channels.json, /sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_channels.json,/sub-01/ses-post/sub-01_ses-post_channels.json,/sub-01/ses-post/sub-01_ses-post_task-seizure_channels.json,/sub-01/sub-01_channels.json,/sub-01/sub-01_task-seizure_channels.json,/channels.json,/task-seizure_channels.json,/sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-02_channels.json
		./sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-03_channels.tsv
			Evidence: Columns: reference, group not defined, please define in: /sub-01/ses-post/ieeg/sub-01_ses-post_channels.json, /sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_channels.json,/sub-01/ses-post/sub-01_ses-post_channels.json,/sub-01/ses-post/sub-01_ses-post_task-seizure_channels.json,/sub-01/sub-01_channels.json,/sub-01/sub-01_task-seizure_channels.json,/channels.json,/task-seizure_channels.json,/sub-01/ses-post/ieeg/sub-01_ses-post_task-seizure_run-03_channels.json
        Summary:                 Available Tasks:        Available Modalities: 
        42 Files, 33.17KB        seizure                 T1w                   
        1 - Subject                                      ieeg                  
        2 - Sessions                                                           
If you have any questions please post on https://neurostars.org/tags/bids

choldgraf · 2019-02-11T16:35:23Z

sorry, all of my cycles had gone towards getting the BIDS-iEEG paper submitted. I can try to take a stab at finishing up the validator code. Then we should re-run on this PR and see what modifications need to be made to ensure this dataset passes the validator

choldgraf · 2019-02-13T17:20:53Z

Just a note that updates are now merged into the validator, so we should see how it works on this dataset once more. Looking at those errors:

The invalid JSON file error should be fixed now
The "empty files" errors can be fixed by just writing in some text into those files (e.g. with open('myfile.vhdr', 'w') as ff: ff.write('some random text'). It's a bit hacky but I think it should work
The "undocumented columns" warning will be smaller (since we now expect x, y, z, size, etc) though you'll need to provide a JSON file to explain the remaining columns.

sappelhoff

Hi @ftadel - based on the experiences in #138 I am positive that we can finally make your PR work and merge it.

So if you have time to go through the final issues, I'd be happy to help and then merge :-)

ftadel · 2019-03-06T20:49:40Z

Passed!

sappelhoff · 2019-03-07T07:24:58Z

Thanks @ftadel ! :-)

ftadel added 2 commits December 3, 2018 00:26

Added epilepsy example dataset

9928a77

Fixed bids specs version

6e20d58

sappelhoff reviewed Dec 3, 2018

View reviewed changes

ftadel added 2 commits December 3, 2018 09:16

Various fixes

72abc99

Various fixes

dbd4bcf

ftadel added 2 commits December 3, 2018 21:12

Add participants.json for fields 'sex' and 'age'

33c7479

Fixed participants.json

199ca43

sappelhoff approved these changes Dec 4, 2018

View reviewed changes

sappelhoff changed the title ~~Added epilepsy example dataset~~ [MRG] Added epilepsy example dataset Dec 4, 2018

sappelhoff added the iEEG label Dec 4, 2018

sappelhoff requested review from dorahermes and choldgraf December 5, 2018 07:56

sappelhoff mentioned this pull request Dec 6, 2018

debug validator for ieeg (bep010): order of columns in channels.tsv bids-standard/legacy-validator#666

Closed

ftadel added 3 commits December 24, 2018 12:19

Fix electrode size: use area (2*pi*r*len=2*pi*0.5*2=6.3 mm2) instead …

ba86eef

…of contact length (len=2mm)

Fix electrode size: 2*pi*r*len=2*pi*0.4*2=6.3 mm2

e837bc8

Events.tsv: Renamed column event_type into trial_type

e288337

sappelhoff self-requested a review March 1, 2019 07:50

sappelhoff reviewed Mar 1, 2019

View reviewed changes

sappelhoff changed the title ~~[MRG] Added epilepsy example dataset~~ [WIP] Added epilepsy example dataset Mar 4, 2019

sappelhoff self-requested a review March 4, 2019 17:21

ftadel added 2 commits March 6, 2019 21:29

Adding dervatives dataset_description.json

13c6024

Fixed typo in sub-01_ses-post_coordsystem.json

6997391

ftadel mentioned this pull request Mar 6, 2019

iEEG examples to add #131

Closed

ftadel added 4 commits March 6, 2019 22:11

Renamed session post into postimp

39ae89e

Added ieeg_epilepsy openneuro link

8b38e97

Add space tag to coordsystem.json

4774a8d

Fixed coordsystem.json and electrodes.tsv

c06e475

sappelhoff approved these changes Mar 7, 2019

View reviewed changes

sappelhoff changed the title ~~[WIP] Added epilepsy example dataset~~ [MRG] Added epilepsy example dataset Mar 7, 2019

sappelhoff merged commit 90a0b7c into bids-standard:bep010_ieeg Mar 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG] Added epilepsy example dataset #133

[MRG] Added epilepsy example dataset #133

ftadel commented Dec 3, 2018

sappelhoff left a comment

sappelhoff Dec 3, 2018

sappelhoff Dec 3, 2018

sappelhoff Dec 3, 2018

sappelhoff Dec 3, 2018

ftadel commented Dec 3, 2018 •

edited

Loading

sappelhoff commented Dec 3, 2018

ftadel commented Dec 4, 2018

sappelhoff left a comment

ftadel commented Dec 5, 2018

sappelhoff commented Dec 5, 2018

chrisgorgo commented Dec 5, 2018 via email

sappelhoff commented Dec 5, 2018

ftadel commented Feb 11, 2019

sappelhoff commented Feb 11, 2019

choldgraf commented Feb 11, 2019

choldgraf commented Feb 13, 2019

sappelhoff left a comment

ftadel commented Mar 6, 2019

sappelhoff commented Mar 7, 2019

[MRG] Added epilepsy example dataset #133

[MRG] Added epilepsy example dataset #133

Conversation

ftadel commented Dec 3, 2018

sappelhoff left a comment

Choose a reason for hiding this comment

sappelhoff Dec 3, 2018

Choose a reason for hiding this comment

sappelhoff Dec 3, 2018

Choose a reason for hiding this comment

sappelhoff Dec 3, 2018

Choose a reason for hiding this comment

sappelhoff Dec 3, 2018

Choose a reason for hiding this comment

ftadel commented Dec 3, 2018 • edited Loading

sappelhoff commented Dec 3, 2018

ftadel commented Dec 4, 2018

sappelhoff left a comment

Choose a reason for hiding this comment

ftadel commented Dec 5, 2018

sappelhoff commented Dec 5, 2018

chrisgorgo commented Dec 5, 2018 via email

sappelhoff commented Dec 5, 2018

ftadel commented Feb 11, 2019

sappelhoff commented Feb 11, 2019

choldgraf commented Feb 11, 2019

choldgraf commented Feb 13, 2019

sappelhoff left a comment

Choose a reason for hiding this comment

ftadel commented Mar 6, 2019

sappelhoff commented Mar 7, 2019

ftadel commented Dec 3, 2018 •

edited

Loading