Skip to content

[WIP] merge bep006_eeg branch into master #95

Closed
robertoostenveld wants to merge 98 commits intomasterfrom
bep006_eeg
Closed

[WIP] merge bep006_eeg branch into master #95
robertoostenveld wants to merge 98 commits intomasterfrom
bep006_eeg

Conversation

@robertoostenveld
Copy link
Copy Markdown
Collaborator

@robertoostenveld robertoostenveld commented May 30, 2018

closes #132

This PR serves to initiate the final stages in the EEG example development. The bep006_eeg branch has some examples that all seem to pass the bids-validator (i.e. the eeg version of it).

This branch should NOT yet be merged as is, because the travis.yml needs to be updated to ensure that it points to the right validator. But creating this PR should hopefully show that the examples are properly aligned.

robertoostenveld and others added 30 commits July 7, 2017 09:28
use "name" and "description" for the columns containing A1 and Fp1 etc.
Data set (empty) from a single subject recording.
Adding empty example data set to EEG branch
fix missing column in participants.tsv
FIX #66 - made a clean version of the EEG-only branch
update bep006_eeg to synchronize with master
I have convertered the original data to EDF (at least pretended to convert it, since I don't have the data). I added the sidecar files. I am not sure whether it is now correct, I suspect there are things missing, but it should be much closer to a BIDS dataset now.
@sappelhoff
Copy link
Copy Markdown
Member

I suggest that we do not include eeg_sleep and eeg_ds000117 with the EEG examples (i.e., delete them before merging this branch)

--> eeg_sleep is incomplete and is not being updated - furthermore there will be no "complete" data to back it up

--> eeg_ds000117 is a fork of the already existing ds000117 and does not really showcase something new.


We'll then end up with having three full datasets:

  • eeg_matchingpennies
  • eeg_rishikesh
  • eeg_rest-fmri

Opinions @CPernet @robertoostenveld ?

@CPernet
Copy link
Copy Markdown
Contributor

CPernet commented Oct 31, 2018 via email

@sappelhoff
Copy link
Copy Markdown
Member

@robertoostenveld do you know more about the "cuba resting state" dataset? And apart from that, are you +1 on removing the incomplete sleep example until @ChristophePhillips finds the time to contribute a complete dataset?

@sappelhoff sappelhoff added the EEG label Nov 20, 2018
@sappelhoff
Copy link
Copy Markdown
Member

looking very good! We only have eeg_rishikesh which is failing --> This will be updated soon by @arnodelorme ... once that is done, I think we can merge the BEP006 examples into master.

I suggest using a squash and merge approach, so that we create a single commit out of all these changes. There was a lot of to and fro in the commit history that we should not pollute the master branch with.

sappelhoff and others added 2 commits January 10, 2019 13:44
Co-authored-by: Arnaud Delorme <arnodelorme@gmail.com>
@robertoostenveld
Copy link
Copy Markdown
Collaborator Author

I recall that there was a "cuba resting state" dataset, but don't really know more about. I think the problem was that nobody currently involved really knows enough details for a good conversion.

I think we are better off (initially) with a few sound examples, rather than with many sloppy ones. We can always extend/improve the examples later (not the ones that are mentioned in the paper, which are important right from the start).

@robertoostenveld
Copy link
Copy Markdown
Collaborator Author

Oh, the Cuban dataset is "eeg_cbm" in the bep006_eeg branch. Looks clean enough to me for inclusion.

+1 for leaving the sleep dataset from @ChristophePhillips out.

@sappelhoff
Copy link
Copy Markdown
Member

I am closing this one in favor of #149, which is the rebased version of this PR. I think that is necessary because of two reasons:

  1. it allows us to directly rebase our changes onto master instead of creating a merge commit (the history becomes more nicely searchable in my opinion)
  2. it allowed me to squash some several small commits into single commits, thereby reducing the bloating

The contents are exactly the same

@sappelhoff sappelhoff closed this Mar 4, 2019
@sappelhoff sappelhoff deleted the bep006_eeg branch March 11, 2019 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

To Do for merging bep006 examples

5 participants