Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove duplicate entity definitions in specification text #612

Open
tsalo opened this issue Sep 17, 2020 · 4 comments
Open

Remove duplicate entity definitions in specification text #612

tsalo opened this issue Sep 17, 2020 · 4 comments
Labels
consistency Spec is (potentially) inconsistent

Comments

@tsalo
Copy link
Member

tsalo commented Sep 17, 2020

In #568 we created a new appendix with the entity definitions. Whenever the entities are referenced in the text, a hyperlink is used to point to the appropriate section of the appendix. The appendix definitions should be considered the "canonical" definitions for these entities, with text in the rest of the specification used only to provide modality-specific context and examples. We did not, however, edit the specification text much in #568, so there are a lot of duplicate definitions and elaborations that could be removed or at least cut down.

@Remi-Gau
Copy link
Collaborator

Was thinking I could do some of that.

Just to be sure: any mention of any of the entities from the appendix should have a hyperlink?

But only if it is "on its own" and not in an example of a "path".

For example:

Add a hyperlink to sub-<label> in the example below.

The purpose of this RECOMMENDED file is to describe properties of participants
such as age, sex, handedness.
In case of single-session studies, this file has one compulsory column
`participant_id` that consists of `sub-<label>`, followed by a list of optional
columns describing participants.
Each participant MUST be described by one and only one row.

But obviously not sub-<label>[_ses-<label>][_acq-<label>][_run-<index>]_phasediff.json

In this particular case, the sidecar JSON file
`sub-<label>[_ses-<label>][_acq-<label>][_run-<index>]_phasediff.json`
MUST define the time of two echos used to map the phase and finally calculate
the phase-difference map.

@tsalo
Copy link
Member Author

tsalo commented Dec 28, 2020

I wish we could render code snippets with hyperlinks, but I don't think it's possible in Markdown (it was something I looked into for #610).

In addition to the linking, there are actual definitions that could be eliminated or reduced, such as this one in MEG:

The proc- entity is analogous to rec- for MR and denotes a variant of a file that was a result of particular processing performed on the device. This is useful for files produced in particular by Elekta’s MaxFilter (for example, sss, tsss, trans, quat, mc), which some installations impose to be run on raw data because of active shielding software corrections before the MEG data can actually be exploited.

I don't know what the updated text should be, but since the description in the text is a complete duplicate of the definition of the entity, it would be great to reduce it. I think the main text should provide some section-specific context, rather than fully defining the entity. Plus, duplications in the text will make it harder to update entity definitions, since we'll have multiple places we have to change.

@tsalo
Copy link
Member Author

tsalo commented Aug 17, 2022

Here is a gist that may make it possible to add hyperlinks to code blocks: https://gist.github.com/jesstelford/cb4dd6fafc18221ce27250e84fd19327

@Remi-Gau
Copy link
Collaborator

probably needs another pass to see how many duplicates are left

@Remi-Gau Remi-Gau added the consistency Spec is (potentially) inconsistent label Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consistency Spec is (potentially) inconsistent
Projects
None yet
Development

No branches or pull requests

2 participants