[ENH] Merge bep006 and bep010

[sappelhoff]

related to #107

This is work in progress [WIP]. I started by updating some configuration files and getting everything ready to add actual content.

Reviews

So far we have approving reviews from

• @arnodelorme
• @gllmflndn
• @chrisfilo
• @robertoostenveld
• @dorahermes
• @choldgraf
• @CPernet

outstanding:

• @ChristophePhillips

To DO:

• add actual content of BEP006
• how to update the section numbers? I suppose we want to squeeze EEG in right after MEG, so all section numberss afterwards will be incremented by one?
• link to EEG examples from spec ... so that users can see practical applications and use it as inspiration
• improve internal linking to other .md documents in the overall specification
• document introduction of new "non-custom" keywords for events.tsv files: value and sample
• for the column headings in the TSV files we use "Field name" (just like for the JSON files). change into "Column name" ... also in MEG spec.
• find consensus about how to format TSV examples: with true tab or with spaces. See and vote here: ascii tables with tabs in markdown are not rendered properly in html sappelhoff/bids-specification#3 (comment)
• discuss about _proc-<label> field, see: 5ff82bd and [ENH] Merge bep006 and bep010 #108 (comment)

How to help

You can directly make changes by applying the following steps:

1. cd bids-specification ... move to your git clone of the bids-specification
2. git remote add stefanappelhoff https://github.com/sappelhoff/bids-specification ... to add my fork of the repository to the "index" of your local clone
3. git fetch stefanappelhoff merge_bep006:merge_bep006 ... pull by branch for merging bep006 into your repository (no merging will happen, this is safe)
4. git checkout merge_bep006 ... checkout "my" branch (as in @sappelhoff's branch ... now also your branch)
5. ... now comes your actual work that you want to do
   1. make a branch (starting from merge_bep006 branch): git checkout -b MYNAME (whatever name you choose for MYNAME)
   2. do your changes ...
   3. push your changes using git push -u origin MYNAME ... this will push the branch to your own clone of the bids-specification repository
   4. and finally, you can do a pull request on https://github.com/sappelhoff/bids-specification
   5. then I will review the request, and merge it ... and it will magically show up here, on bids-standard/bids-specification. :-)

Using this method, you can also stay up to date with the changes that I do while working on this PR ... do it through:

1. git checkout merge_bep006
2. git pull stefanappelhoff merge_bep006

sounds easy right?! (let me know if I described something in the wrong way)

No longer applicable

[sappelhoff]

We need to discuss how to deal with certain parts of the specification that are shared between EEG, MEG, and iEEG:

for example:

• photo.jpg
• The restricted list of channel type keywords

... one option is to duplicate the information within each modality specific page.
... another option is to create pages that summarize across several modalitites (e.g., MEG, EEG, iEEG)
... any idea for other options?

Personally, I am tending towards the duplication of information within each modality specific site for two reasons:

1. it will make the page easily browsable without having to switch pages to obtain some information
2. albeit the largely overlapping information, there might be some subtle differences between modalities that we would have to resolve

What are your takes on this?

@choldgraf @dorahermes @CPernet @robertoostenveld @chrisfilo

[chrisgorgo]

I personally don't mind a little redundancy (sadly markdown does not support including source files). Best, Chris

…

On Wed, Dec 19, 2018 at 3:59 PM Stefan Appelhoff ***@***.***> wrote: We need to discuss how to deal with certain parts of the specification that are shared between EEG, MEG, and iEEG: for example: - photo.jpg - The restricted list of channel type keywords ... one option is to duplicate the information within each modality specific page. ... another option is to create pages that summarize across several modalitites (e.g., MEG, EEG, iEEG) ... any idea for other options? Personally, I am tending towards the duplication of information within each modality specific site for two reasons: 1. it will make the page easily browsable without having to switch pages to obtain some information 2. albeit the largely overlapping information, there might be some subtle differences between modalities that we would have to resolve What are your takes on this? @choldgraf <https://github.com/choldgraf> @dorahermes <https://github.com/dorahermes> @CPernet <https://github.com/CPernet> @robertoostenveld <https://github.com/robertoostenveld> @chrisfilo <https://github.com/chrisfilo> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#108 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOkp1wnZcG5cFfw47rcrbafCRvfCqBsks5u6qicgaJpZM4ZVYHb> .

[robertoostenveld]

For clarity and usability by the end-users I also think that some redundancy is better. But for maintainability it would be better to have it in one place. I don’t know how mkdocs works, but for the fieldtrip website (based on Jekyll/Liquid/Ruby) we do have a form of includes. for example if you take https://github.com/fieldtrip/website/blob/master/tutorial/headmodel_eeg_bem.md <https://github.com/fieldtrip/website/blob/master/tutorial/headmodel_eeg_bem.md> and https://github.com/fieldtrip/website/blob/master/tutorial/headmodel_eeg_fem.md <https://github.com/fieldtrip/website/blob/master/tutorial/headmodel_eeg_fem.md> they both have “{% include /shared/tutorial/sourcelocalization_background.md %}” which is actually https://github.com/fieldtrip/website/blob/master/_includes/shared/tutorial/sourcelocalization_background.md <https://github.com/fieldtrip/website/blob/master/_includes/shared/tutorial/sourcelocalization_background.md> The shared markdown file is included at the build moment. Perhaps something like this would also work for mkdocs. best Robert

…

On 20 Dec 2018, at 00:06, Chris Gorgolewski ***@***.***> wrote: I personally don't mind a little redundancy (sadly markdown does not support including source files). Best, Chris On Wed, Dec 19, 2018 at 3:59 PM Stefan Appelhoff ***@***.***> wrote: > We need to discuss how to deal with certain parts of the specification > that are shared between EEG, MEG, and iEEG: > > for example: > > - photo.jpg > - The restricted list of channel type keywords > > ... one option is to duplicate the information within each modality > specific page. > ... another option is to create pages that summarize across several > modalitites (e.g., MEG, EEG, iEEG) > ... any idea for other options? > > Personally, I am tending towards the duplication of information within > each modality specific site for two reasons: > > 1. it will make the page easily browsable without having to switch > pages to obtain some information > 2. albeit the largely overlapping information, there might be some > subtle differences between modalities that we would have to resolve > > What are your takes on this? > > @choldgraf <https://github.com/choldgraf> @dorahermes > <https://github.com/dorahermes> @CPernet <https://github.com/CPernet> > @robertoostenveld <https://github.com/robertoostenveld> @chrisfilo > <https://github.com/chrisfilo> > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#108 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAOkp1wnZcG5cFfw47rcrbafCRvfCqBsks5u6qicgaJpZM4ZVYHb> > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#108 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA2345OdXjNU3VlnJulXPwDsjbL06QFJks5u6sZ-gaJpZM4ZVYHb>.

[sappelhoff]

@robertoostenveld @CPernet

I have finished copying over and reformatting the BEP006 specification. I did some copy-editing along the way: Some typo fixing, reformulations, and using the text from the paper rather than from the google doc, so it would be great if the two of you could critically go over the text and make comments / suggestions.

After the review, this is ready to be merged from my side!

[chrisgorgo]

Great job!

[choldgraf]

This looks good to me - I think the explanation of "space" is clearer (and thanks for linking to the spec page for conversations around this). I think this is in a good state to merge, and we can iteratively improve things as people have questions over time

[chrisgorgo]

Thanks, @choldgraf. In such case could you go to https://github.com/bids-standard/bids-specification/pull/108/files click "Review changes" and submit an Approve? We need this to unlock merging.

[choldgraf]

[chrisgorgo]

@CPernet it would be great to get a review from you as well.

[chrisgorgo]

I reached out to @CPernet over email, but to no avail. Should we go ahead and merge this?

[choldgraf]

I believe that it satisfies all of the points in the governance doc, so I'm +1 on merging. Perhaps we give it another day.

If we merge and @CPernet finds things that he thinks should be different, I'm happy to iterate w/ folks on a follow-up PR.

[CPernet]

cosmetic changes - thx for doing this

[sappelhoff]

I put @CPernet's points into the code, but this dismissed the approving reviews. We are ready now.

So, please provide approving reviews and then @chrisfilo can merge it :-)

@choldgraf @dorahermes @robertoostenveld

[choldgraf]

I re-added mine - somebody else approve and then let's merge this thing!

[choldgraf]

3 approvals!!! 🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢

[sappelhoff]

According to our new rules, we'd have to wait with a merge for 5 days after the last commit. And since I only merged Cyril's review yesterday, we are still in that period :-/

I don't think that it makes sense though. However, I am not sure how/when or who could make an exception to our self-imposed rules. It might be something to discuss ...

PS: For those wondering about the squirrel that @choldgraf seems to be fond of: https://www.quora.com/On-GitHub-what-is-the-significance-of-the-Ship-It-squirrel ;-)

[chrisgorgo]

Indeed the wait is a little bit annoying. I originally planned to merge this PR quickly (just after @CPernet suggestions) and ask @CPernet to send a new PR - @sappelhoff was, however, quick to make a commit. It does not matter though - the two PR situation would require us to wait another 5 days to merge the second PR.

In general, I think it is worth observing the rules fully (even if some of the might be irritating) for a period of time and try to come up with improvements. There is hope for introducing a dedicated maintainer to the project that could get superpowers to deal with such edge situations.

[sappelhoff]

[chrisgorgo]

Congratulations everyone! 🎉

[choldgraf]