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.

Sign up for GitHub

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

Jump to bottom

[FIX] Typos and clarifications #386

Merged
merged 3 commits into from
Jan 13, 2020
Merged

[FIX] Typos and clarifications #386

merged 3 commits into from
Jan 13, 2020

Conversation

apjanke
Copy link
Contributor

@apjanke apjanke commented Dec 21, 2019 •
edited
Loading

Just some suggested minor typo and style fixes.

@apjanke apjanke requested a review from chrisgorgo as a code owner December 21, 2019 03:29
apjanke
apjanke commented Dec 21, 2019
View reviewed changes
src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Outdated
@@ -290,7 +290,7 @@ combined image rather than an image from each coil.
| NumberOfVolumesDiscardedByScanner | RECOMMENDED. Number of volumes ("dummy scans") discarded by the scanner (as opposed to those discarded by the user post hoc) before saving the imaging file. For example, a sequence that automatically discards the first 4 volumes before saving would have this field as 4. A sequence that doesn't discard dummy scans would have this set to 0. Please note that the onsets recorded in the \_event.tsv file should always refer to the beginning of the acquisition of the first volume in the corresponding imaging file - independent of the value of `NumberOfVolumesDiscardedByScanner` field. |
| NumberOfVolumesDiscardedByUser | RECOMMENDED. Number of volumes ("dummy scans") discarded by the user before including the file in the dataset. If possible, including all of the volumes is strongly recommended. Please note that the onsets recorded in the \_event.tsv file should always refer to the beginning of the acquisition of the first volume in the corresponding imaging file - independent of the value of `NumberOfVolumesDiscardedByUser` field. |
| DelayTime | RECOMMENDED. User specified time (in seconds) to delay the acquisition of data for the following volume. If the field is not present it is assumed to be set to zero. Corresponds to Siemens CSA header field `lDelayTimeInTR`. This field is REQUIRED for sparse sequences using the `RepetitionTime` field that do not have the `SliceTiming` field set to allowed for accurate calculation of "acquisition time". This field is mutually exclusive with `VolumeTiming`. |
| AcquisitionDuration | RECOMMENDED. Duration (in seconds) of volume acquisition. Corresponds to DICOM Tag 0018,9073 `Acquisition Duration`. This field is REQUIRED for sequences that are described with the `VolumeTiming` field and that not have the `SliceTiming` field set to allowed for accurate calculation of "acquisition time". This field is mutually exclusive with `RepetitionTime`. |
| AcquisitionDuration | RECOMMENDED. Duration (in seconds) of volume acquisition. Corresponds to DICOM Tag 0018,9073 `Acquisition Duration`. This field is REQUIRED for sequences that are described with the `VolumeTiming` field and that do not have the `SliceTiming` field set to allowed for accurate calculation of "acquisition time". This field is mutually exclusive with `RepetitionTime`. |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed "and that not have" to "and that do not have"

sappelhoff
sappelhoff previously approved these changes Jan 6, 2020
View reviewed changes
Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Thanks @apjanke these changes look good to me.

Have you considered adding yourself to the contributors list?

see: https://github.com/bids-standard/bids-specification/blob/master/src/99-appendices/01-contributors.md

If you want to, you can just do one additional commit here.

effigies
effigies reviewed Jan 7, 2020
View reviewed changes
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.

Looks good. A couple fixes for the fixes.

src/02-common-principles.md Outdated Show resolved Hide resolved
src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Outdated Show resolved Hide resolved
@effigies
Copy link
Collaborator

effigies commented Jan 7, 2020

The easiest way to add yourself as a contributor is to edit the Wiki page (https://github.com/bids-standard/bids-specification/wiki/Contributors). This will be merged in before any release.

And to be clear, BIDS strongly encourages any level of engagement, so let me assure you that you have contributed enough to add yourself to the list.

@apjanke @effigies
Typo fix for typo fix
2faf7f8 
Co-Authored-By: Chris Markiewicz <effigies@gmail.com>
@apjanke @effigies
Typo fix for typo fix
2302eee 
Co-Authored-By: Chris Markiewicz <effigies@gmail.com>
@apjanke
Copy link
Contributor Author

apjanke commented Jan 7, 2020

LOL. Fixes for the fixes pulled in. And I've added myself to the Contributors wiki page. Thanks!

@effigies effigies changed the title [FIX] Minor typo fixes [FIX] Typos and clarifications Jan 7, 2020
@sappelhoff sappelhoff merged commit 21480a9 into bids-standard:master Jan 13, 2020
@sappelhoff
Copy link
Member

Thanks @apjanke!

franklin-feingold added a commit that referenced this pull request Jan 13, 2020
@franklin-feingold
[DOC] Auto-generate changelog entry for PR #386
426abcd
satra added a commit to satra/bids-specification that referenced this pull request Feb 7, 2020
@satra
Merge remote-tracking branch 'upstream/master' into units2ascii
a99e304 
* upstream/master: (439 commits)
  [DOC] Auto-generate changelog entry for PR bids-standard#394
  Update 01-contributors.md
  [DOC] Auto-generate changelog entry for PR bids-standard#389
  [DOC] Auto-generate changelog entry for PR bids-standard#386
  moved list of extension proposals to the main BIDS website section 06-extensions remains, but now points to the main website
  Typo fix for typo fix
  Typo fix for typo fix
  [DOC] Auto-generate changelog entry for PR bids-standard#374
  [FIX] Fix some typos and prose style issues
  [DOC] Auto-generate changelog entry for PR bids-standard#383
  restructure according to @emdupre's proposal
  neurostars for everything not just technical
  Apply suggestions from code review
  MNT: Update Pipfile
  ENH: Append CSS instead of overriding it
  ENH: Add watermark to drafts
  [DOC] Auto-generate changelog entry for PR bids-standard#381
  Update src/06-extensions.md
  Retiring moderator duties for BEP021
  [DOC] Auto-generate changelog entry for PR bids-standard#380
  ...
satra added a commit to satra/bids-specification that referenced this pull request Feb 7, 2020
@satra
Merge remote-tracking branch 'upstream/master' into units2ascii
9622fdc 
* upstream/master: (439 commits)
  [DOC] Auto-generate changelog entry for PR bids-standard#394
  Update 01-contributors.md
  [DOC] Auto-generate changelog entry for PR bids-standard#389
  [DOC] Auto-generate changelog entry for PR bids-standard#386
  moved list of extension proposals to the main BIDS website section 06-extensions remains, but now points to the main website
  Typo fix for typo fix
  Typo fix for typo fix
  [DOC] Auto-generate changelog entry for PR bids-standard#374
  [FIX] Fix some typos and prose style issues
  [DOC] Auto-generate changelog entry for PR bids-standard#383
  restructure according to @emdupre's proposal
  neurostars for everything not just technical
  Apply suggestions from code review
  MNT: Update Pipfile
  ENH: Append CSS instead of overriding it
  ENH: Add watermark to drafts
  [DOC] Auto-generate changelog entry for PR bids-standard#381
  Update src/06-extensions.md
  Retiring moderator duties for BEP021
  [DOC] Auto-generate changelog entry for PR bids-standard#380
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sappelhoff sappelhoff sappelhoff approved these changes

@effigies effigies effigies approved these changes

@chrisgorgo chrisgorgo Awaiting requested review from chrisgorgo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@apjanke @effigies @sappelhoff