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

Re-organizing suffix section and other improvements [WIP] #59

Open
wants to merge 50 commits into
base: master
Choose a base branch
from

Conversation

agahkarakuzu
Copy link
Collaborator

@agahkarakuzu agahkarakuzu commented May 29, 2019

  • Improved PCTS for suffix
    • Added an explanation on why minimizing the use of weight tags for grouping qMRI inputs is important
    • Re-organized suffix justification section w.r.t. thesuffix classes.
    • Added principles for adding a new suffix.
    • Added answers to some plausible questions under suffix section, specifically for MPM & MTS convention.
  • Improved main spec
    • Re-structure suffix section in the specification: Created 3 classes

    • Updated tables and explanations wrt 3 classes

    • Added rules for grouping suffix,indexable_metadata and acq-<label>

    • Added new text for elaborating on JSON files accompanying grouping suffixes

      • Added method specific priority levels table to relate grouping suffixes with metadata fields
      • Added another table for qMRI applications that can be derived from an existing grouping suffix by means of adding/modifying metadata priority levels.
  • TODO
    • parent & child TO BE CHANGED into high & low level, or some other proper pair
    • Take session-<> into account in the explanation of the higher level JSON for grouping suffix
    • Complete B1DAM suffix.
    • Complete PCTS for indexable_metadata, acq and JSON fields.

@agahkarakuzu agahkarakuzu changed the title Indexable metadata [WIP] Re-organizing suffix section and other improvements [WIP] May 29, 2019
@agahkarakuzu agahkarakuzu added the feedback requested Please speak now label May 29, 2019
@KirstieJane KirstieJane self-assigned this Jul 25, 2019
@KirstieJane KirstieJane changed the base branch from master to repetitiontime August 15, 2019 16:40
@KirstieJane KirstieJane changed the base branch from repetitiontime to master August 15, 2019 16:40
@KirstieJane
Copy link
Member


>>>> TO BE DISCUSSED START
__


Good practice recommendations:

Parameters that are unchanged across the constituents of a
grouped scan collection MAY repeat in every JSON file belonging to that group.
To avoid such redundancy, all the fixed metadata fields MAY be included only
in the JSON file of the first member of a grouped scan collection, where
constituent images are sorted in ascending alphabetical order. If this convention
is followed, then the remaining JSON files MAY only contain parameters that are
subjected to at least one change across all the member images.


TO BE DISCUSSED END <<<<


@Gilles86 and I are reviewing this PR and although we can see the benefits of not having to write out the same metadata over and over, we both agree that this proposal breaks the inheritance principle and ends up being much more complicated to implement 😭

I've deleted it for now....sorry @agahkarakuzu! I hope that's ok 😬

* Improve introductory text 
* Add two columns to clearly describe REQUIRED and OPTIONAL naming entities for grouping suffixes 
* Add GRASE 
* Add rfphase to the indexable_metadata list.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback requested Please speak now
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants