State of the BEP (Do not merge)#6
Closed
effigies wants to merge 1237 commits intocommon-derivativesfrom
Closed
Conversation
This was referenced Aug 1, 2019
Collaborator
Easier to just change the target branch on existing PR #5; there's no conflicts, so the branch change can be easily dealt with at merge time rather than rebasing commits. |
oesteban
approved these changes
Aug 13, 2019
Collaborator
|
@Lestropie, the rebase is proposed to make it easier for reviewers to see the differences that are not already on the target branch, correct @effigies ? |
Collaborator
|
GitHub is clever enough to show the online diff as the changes that will be applied to the target branch as a result of the merge, rather than all differences between the two branches. So #5 should still only show changes that are relevant to that proposal. |
Contributor
Author
|
@Lestropie, yes, if the deleting and re-creating of the DWI sections of the spec didn't produce a merge conflict, then there's no need to use the rebased branch. I wasn't really sure how well git would handle that case, so I tend toward the over-cautious. |
* Added full names for some contributors in mailmap * add more names that we know * one more Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>
* [ENH] Bep 005: Arterial Spin Labeling (#652) Co-authored-by: Patricia Clement <41481345+patsycle@users.noreply.github.com> Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org> Co-authored-by: Chris Markiewicz <effigies@gmail.com> Co-authored-by: Julia Guiomar Niso Galán <guiomar.niso@ctb.upm.es> Co-authored-by: Remi Gau <remi_gau@hotmail.com> * Update table errors * correction for table issues 'common metadata for PCASL/PASL' * correction latin phrases * Corrections tables * correction tables Cases * added link pepolar * deleted 'don't discriminate between types of labeling based on comment Thijs Van Osch * Moved MagneticFieldStrenght asl requirement to common * removed scaling factor info based on comment gllmflndn Removed: all ancillary scaling factors should be taken into account in the conversion to BIDS, which is why BIDS does not provide a separate scaling factor field other than the NIfTI header. * added MRAcquisitionType added in the common - sequence specifics table Reason: for ASL, defining 2D and 3D is required. This used to be added in the PulseSequenceType field, but this should stay a free text field. Therefore, the MRAcquisitionType field is added (based on DicomTag). * removed PulseSequenceType from the ASL part The 2D/3D information needed for ASL, is now moved to the new MRAcquisitionType field. * added required common fields sentence Some Common fields are required for ASL. To stress this out, we added a sentence for the *_as.json and the *_m0scan.json. * adaptation requirements for m0scan.json * adapted requirement level common fields for m0scan * update sentence required common fields for asl.json * Update common field EchoTime * Removed EchoTime from ASL-table * update FlipAngle in common RF and Contrasts * remove FlipAngle from ASL fields * update Dependency table ASL for MRAcquisitionType * update SliceTiming in common fields * remove SliceTiming from ASL tables * added (FA) for FlipAngle * update common RF and Contrast for table check * LabelingPulseDuration and LabelingPulseInterval in ms instead of s * required common metadatafields update RepetitionTime or VolumeTiming to RepetitionTimePreparation * update flipangle for timeseries versus file collections * update echotime for timeseries * update timing table (fence) * update definition BackgroundSuppressionPulseTime * removed common metadata fields from asl table MagneticFieldStrenght, EchoTime, SliceTiming, VolumeTiming, RepetitionTime, FlipAngle, PulseSequenceType * update scaling of asl and m0 files - update scaling description for nifti files - created separate scaling section (to highlight this important quantification factor), and M0 section - moved scaling section and M0 section after aslcontext section * update broken link in EchoTime (Timing Parameters) * update broken link (2) EchoTime * Update broken links FlipAngle * LabelingType changed to ArterialSpinLabelingType - changed in fields - Changed in dependency table - Dependency table: additionally removed repetitiontime/volumetiming parts * Update dependency table based on new M0Type and M0Estimate fields * Update new M0 field strategy M0 field becomes M0Type, with addition field M0Estimate. Also corrected IntendedFor description in m0 specific fields * TEMPORARY: disable 'strict' docs build this allows the build to proceed despite broken links * changed fail_on_warning to true Intermediate solution to get BEP005 build untill the missing link is available * see previous commit * update bep005 with latest master + resolve conflict (#689) * [DOC] Auto-generate changelog entry for PR #677 * [ENH] BEP001 - New entities: inv & mt (#681) * add entity inv * add mt: * add MT specific metadata * Generate entity.md * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org> * Update src/schema/entities.yaml Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org> * table fix by @sappelhoff * Conditional requirement Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org> * [ENH] BEP001 - Entity-linked file collections (#688) * Squashed commit of the following: commit 8e1ac5fe358691f34ca1c2b57b5812d2dc602173 Author: Stefan Appelhoff <stefan.appelhoff@mailbox.org> Date: Fri Nov 27 11:09:24 2020 +0100 fix links and latin commit 27c9b41b1d1cedc38654db39b3fd5e573b8ce660 Author: Stefan Appelhoff <stefan.appelhoff@mailbox.org> Date: Fri Nov 27 10:03:51 2020 +0100 remove latin Co-authored-by: Chris Markiewicz <effigies@gmail.com> commit 7d87070ecb6eb3636d5b15e304b8e65de2b8c93b Author: Agah <agahkarakuzu@gmail.com> Date: Mon Nov 23 08:46:47 2020 -0500 Update src/99-appendices/10-file-collections.md Co-authored-by: Chris Markiewicz <effigies@gmail.com> commit b137a9e3cd2bce7cfe7853909684267d4689b692 Author: Agah <agahkarakuzu@gmail.com> Date: Mon Nov 23 08:45:19 2020 -0500 Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <effigies@gmail.com> commit d7897b8259213dd1cfb6e1db4ec835116491bfb1 Author: Agah <agahkarakuzu@gmail.com> Date: Mon Nov 23 08:44:40 2020 -0500 Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <effigies@gmail.com> commit 09e9d32b033a076a8eeb8408d5e62798bb3629ed Author: Agah <agahkarakuzu@gmail.com> Date: Mon Nov 23 08:44:15 2020 -0500 Commit suggestion Co-authored-by: Chris Markiewicz <effigies@gmail.com> commit 009a9ab1563020c130553253480ef76467d894c2 Author: Agah Karakuzu <agahkarakuzu@gmail.com> Date: Mon Nov 23 08:43:00 2020 -0500 Add file-collecitons appendix to the TOC commit 8f6c4dfbe7f79232cadca72f0fd22b693a238c52 Author: Agah Karakuzu <agahkarakuzu@gmail.com> Date: Mon Nov 23 08:41:13 2020 -0500 Address suggestion by @tsalo commit 45210a9d167a7532c1ab8463c62dbbc519969c95 Author: Agah Karakuzu <agahkarakuzu@gmail.com> Date: Wed Nov 11 14:32:20 2020 -0500 Wording commit 66ba5a5313d4951c826063de13da2d60363b6edd Author: Agah Karakuzu <agahkarakuzu@gmail.com> Date: Wed Nov 11 14:31:40 2020 -0500 RECOMMENDED --> MUST for adding an application def to the appdx commit 5611fac6a365eef07acf36290cbf12f22a1d4751 Author: Agah Karakuzu <agahkarakuzu@gmail.com> Date: Wed Nov 11 14:29:21 2020 -0500 Improve the appendix commit 6d3bbeadc464ec4a93f97d96d3cffdad8f920f29 Author: Agah Karakuzu <agahkarakuzu@gmail.com> Date: Mon Nov 2 16:12:04 2020 -0500 Address suggestions by @tsalo commit bcb02232ca769f14a11eb8b056bb963591901012 Author: Agah Karakuzu <agahkarakuzu@gmail.com> Date: Tue Oct 27 21:37:26 2020 -0400 [ADD] Link to the appendix commit 44e760f69163eb11bec74be58279bd260aad963c Author: Agah Karakuzu <agahkarakuzu@gmail.com> Date: Tue Oct 27 21:28:38 2020 -0400 [ADD] Appendix - File collections - For parametrically linked file collections commit 523b1c163df4b5a1d46f25156d1b0ea0d06ac7bb Author: Agah Karakuzu <agahkarakuzu@gmail.com> Date: Tue Oct 27 21:27:52 2020 -0400 [ADD] Parametrically linked file collections - Description * ENH: Link to all BEP-001 entities * STY: Escape asterisk, adjust table widths * Add suggestions by @sappelhoff * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <effigies@gmail.com> * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <effigies@gmail.com> * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <effigies@gmail.com> * Update src/99-appendices/10-file-collections.md Co-authored-by: Chris Markiewicz <effigies@gmail.com> * Update src/02-common-principles.md Thank you @effigies! Co-authored-by: Chris Markiewicz <effigies@gmail.com> * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <effigies@gmail.com> Co-authored-by: Agah Karakuzu <agahkarakuzu@gmail.com> * [ENH] BEP001 - RepetitionTimeExcitation and RepetitionTimePreparation (#671) * [ENH] Improve TR definitions - Include RepetitionTimePreparation - Include RepetitionTimeExcitation - Add explanation on the former RepetitionTime field. * Typo * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <effigies@gmail.com> * ENH: Allow for variable RTE/P * STY: Update table widths * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md * Change ref to DOI @Remi-Gau * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <effigies@gmail.com> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <effigies@gmail.com> Co-authored-by: Chris Markiewicz <effigies@gmail.com> Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> * Update config.yml * Update readthedocs.yml Co-authored-by: bids-maintenance <bids.maintenance@gmail.com> Co-authored-by: Agah <agahkarakuzu@gmail.com> Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> Co-authored-by: Chris Markiewicz <effigies@gmail.com> * update required common fields for m0scan.json added EchoTime and FlipAngle in case LookLocker is true * update dependency table if PCASL : LabelingDuration is required if PCASL: LabelingDuration should not be filled in * update depedency table removed 'PASL' LAbelingDuration should not be filled in * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> * update description aslcontext Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> * update PCASLType and CASLType Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> * updated required into REQUIRED for asl.json and m0scan.json (common fields * Added SummaryImages Added images: asl_pasl_boluscutoff_false.png asl_pasl_boluscutoff_true_q2tips.png asl_pasl_boluscutoff_true_quipssII.png asl_pcasl_labeling_pulses.png asl_pcasl_sequence.png * create Appendix 11 for ASL * Added asl_flowchart.png * added subtitles for linking from main specification * Added appendix link for control and label * added appendix for asl sequences in general * added appendix for (P)CASL * added appendix for PASL * added appendix for dependency table (flowcharts) * MNT: Add ASL appendix to TOC * TYPO: correct ASL appendix entry in tocwq * fix md style * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> * update table RF and Contrast * update link Appendix XI to Appendix XI - ASL * corrected link to appendix for PASL * Update aslcontext cases Case 1: changed asl.json to asl.nii[.gz`] Removed: The optional deltam or cbf volumes should be stored and specified as derivative. Case 2: Removed: The optional cbf volumes should be stored and specified as derivative. * Added aslcontext.tsv cases this is moved from main spec bep005 to appendix XI - ASL * Update aslcontext cases - moved three cases to appendix XI - ASL - added link to appendix XI - added: Note that the raw images, including the m0scan, may also be used for quality control. * corrected subtitles cases * improve linebreaks in ASL appendix * fix VascularCrushingVENC link Based on comment of Stefan Appelhof, the Data Type for VascularChrushingVENC was fixed: [number][] or \[array][] of [numbers][] to [number][] or [array][] of [numbers][] * added single column requirement for aslcontext.tsv based on comment of Stefan Appelhof, the need for a single column was added in the introductory text for aslcontext.tsv, before the 3 cases. * Changed VascularCrushingVENC to VascularCrushingVenc * [SCHEMA] Add ASL to schema (#703) * Add ASL to schema files and regenerate entity table. * Fix style issue. * Add fmap-format m0scan to schema. * update M0Estimate fields Added 'Referring to the M0 of blood' * update LabelingDuration Added additional information for LabelingDuration * update labeling.jpg into *_asllabeling.jpg * Change labeling to asllabeling. (#709) * RF: Renumber ASL appendix to XII Co-authored-by: Henk Mutsaerts <henkjanmutsaerts@gmail.com> Co-authored-by: Patricia Clement <41481345+patsycle@users.noreply.github.com> Co-authored-by: Chris Markiewicz <effigies@gmail.com> Co-authored-by: Julia Guiomar Niso Galán <guiomar.niso@ctb.upm.es> Co-authored-by: Remi Gau <remi_gau@hotmail.com> Co-authored-by: bids-maintenance <bids.maintenance@gmail.com> Co-authored-by: Agah <agahkarakuzu@gmail.com> Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> Co-authored-by: Taylor Salo <tsalo006@fiu.edu>
Update inversion, resolution, density and description. Skipping mt for now...
Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>
SCHEMA: Update entity YAML keys
* harmonize coordsystems * fix table pipe * remove two typos * fix typo * text clarifications Co-authored-by: Taylor Salo <tsalo006@fiu.edu> Co-authored-by: Taylor Salo <tsalo006@fiu.edu>
* harmonize coordsystems * fix table pipe * remove two typos * fix typo * text clarifications Co-authored-by: Taylor Salo <tsalo006@fiu.edu> Co-authored-by: Taylor Salo <tsalo006@fiu.edu>
this is getting old VERY quickly
"large emoji problem"
[MISC] make table headers bold
[MISC] update available datatypes in specification
[FIX] What is a composite instance? Change to measurement for non MRI modalities?
[FIX] document required column order MEG, EEG, iEEG, PET, and fix typo iEEG
francopestilli
approved these changes
Jun 13, 2021
| # Diffusion derivatives | ||
|
|
||
| ## Preprocessed diffusion weighted images | ||
| ## Preprocessed diffusion-weighted images |
Collaborator
There was a problem hiding this comment.
i belive the dash is consistent with English grammar
| Once the model is fit, the resulting representation can be saved using a number | ||
| of values per voxel. Those values will be called **output** or **estimated | ||
| parameters** in the following. | ||
| Diffusion MRI can be modeled using various paradigms to extract more |
Collaborator
There was a problem hiding this comment.
Would "protocol" instead of "paradigm" here be more appropriate?
https://wikidiff.com/paradigm/protocol#:~:text=As%20nouns%20the%20difference%20between,model%20or%20pattern%3B%20a%20template.
Collaborator
|
Do we need another merge of upstream/master here to see the diff just for the BEP? |
Closed
Collaborator
|
Superseded by #24. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
18 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR exists to compare the current state of the BEP against the
common-derivativesbranch, copied over from the main spec.I created a new branch called
bep-016, and restored the diffusion derivatives at the state they were at when they were removed from common derivatives. The only difference is that the newlines were in DOS mode (\r\n) for some reason, and I converted to UNIX (\n) for consistency with the spec.The strategy I would suggest is: treat the
bep-016branch like yourmasterbranch. I've set it as the default branch, so any new pull requests will default against it. When things are merged into it, this PR will update, and you'll be able to see if there are any conflicts with common derivatives.@Lestropie I've rebased your branch onto this version, and I think it should look very similar:
dwi_derivatives_restructure_rebased.I will open an issue for process discussion.