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

[FIX] Clarify run indexing information for MRI acquisitions #694

Closed
wants to merge 3 commits into from

Conversation

dlevitas
Copy link
Contributor

@dlevitas dlevitas commented Dec 8, 2020

Adding fix, regarding this NeuroStars issue

@dlevitas dlevitas changed the title [FIX] Update run indexing information for MRI acquisitions [FIX] Clarify run indexing information for MRI acquisitions Dec 8, 2020
@tsalo
Copy link
Member

tsalo commented Dec 9, 2020

I don't have time to review this just yet, but I did want to note that this speaks to the need to delineate which entities define a file collection (as in #688) and which ones differentiate runs (#530).

…ata.md

Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>
Copy link
Collaborator

@effigies effigies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there was some ambiguity about my last proposal. I was suggesting replacing the entire paragraph.

Comment on lines 168 to 170
If several scans of the same modality are acquired they MUST be indexed with the
[`run-<index>`](../99-appendices/09-entities.md#run) key-value pair:
`_run-1`, `_run-2`, `_run-3`, and so on (only nonnegative integers are allowed as
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If several scans of the same modality are acquired they MUST be indexed with the
[`run-<index>`](../99-appendices/09-entities.md#run) key-value pair:
`_run-1`, `_run-2`, `_run-3`, and so on (only nonnegative integers are allowed as

run labels). When there is only one scan of a given type the run key MAY be
omitted. Please note that diffusion imaging data is stored elsewhere (see
below).
run labels).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
run labels).

Comment on lines +177 to +180
If different entities apply,
such as a different session indicated by [`ses-<label>`](../99-appendices/09-entities.md#ses),
or different acquisition parameters indicated by [`acq-<label>`](../99-appendices/09-entities.md#acq),
then `run` is not needed to distinguish the scans and MAY be omitted.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is crucial information that should be reflected in the run entity definition, rather than in a specific modality's section.

@effigies
Copy link
Collaborator

effigies commented Feb 2, 2021

Closing in favor of #719.

@effigies effigies closed this Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants