[FIX] improve HED documentation #619
Conversation
This implements HED strings that take values from TSV data as described in bids-standard/bids-specification#619. A new issue is added to account for cases where too many or too few number signs are provided in a string. Tests have been added.
There was a problem hiding this comment.
Thanks @VisLab and @happy5214
I left a few comments --> most of them related to formatting of the sentences. Remember that we try to start a new line for each new sentence, and wherever else it makes sense semantically, so that in the future the git diff will be more readable.
Corrected can/should typo and tried to break lines at sentences.
There was a problem hiding this comment.
Thanks for addressing the fist round of reviews @VisLab
regarding the "spaces" instead of "tabs" in the TSV examples, I am okay with them being spaces. We don't have a policy for that right now, and if we want one, we should discuss it in a separate, dedicated issue.
I left one more comment on what was unclear to me, it'd be nice if you could address that.
Finally, I think we can still include this PR in the 1.4.1 release that's upcoming, so I added a milestone to the PR.
Co-authored-by: Stefan Appelhoff <[email protected]>
sappelhoff
changed the title
|
Thanks @VisLab |
sorry for coming late to the game: by the definition of TSV (tab-separated values) TAB must be used as a separator, or it is not a TSV, and tools reading TSV file might not read it correctly if spaces are used. There is IMHO no need to re-iterate that it must be TAB, like there is no need to provide a full description of JSON syntax while talking about JSON files. |
|
We were talking only about how to format examples in the flow text of the spec. Markdown Codeblocks either containing spaces to look nice or tabs to be accurately reflecting tsv |
|
I changed to spaces because the unaligned columns made the table very
difficult to understand the table. I don't think this implies that tsv
files shouldn't be tab-separated. We could explicitly put an indication
that the table has been solumn-aligned to improve readability. We could
also put a link to a file containing the actual tab separated information.
I don't think anyone actually cuts and pastes these examples into a file.
They should probably be looking at the examples repository for that.
…On Thu, Oct 8, 2020 at 10:46 AM Stefan Appelhoff ***@***.***> wrote:
We were talking only about how to format examples in the flow text of the
spec. Markdown Codeblocks either containing spaces to look nice out tabs to
be accurately reflecting tsv
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#619 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJCJOUWUYAFDX4FZ6XN773SJXNFZANCNFSM4RSKUTGQ>
.
|
This implements HED strings that take values from TSV data as described in bids-standard/bids-specification#619. A new issue is added to account for cases where too many or too few number signs are provided in a string. Tests have been added.
This request makes some editorial improvements in HED.md such as adding subsections. It also adds the specification of how to document in machine-actionable form the meaning of columns in *_events.tsv files containing values rather than categorical values. There will be an accompanying pull request for the bids-validator incorporating the implementation of this feature.