-
Notifications
You must be signed in to change notification settings - Fork 163
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
[ENH] Add res
and den
keywords to indicate resolution of resampled data
#301
Merged
effigies
merged 31 commits into
bids-standard:common-derivatives
from
oesteban:enh/resolution-density
May 22, 2020
Merged
Changes from all commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
dda7aca
fix: force-push a rewrite addressing minimal changes
oesteban e10f451
sty: make linter happy
oesteban 3a85c0a
[STY] Add missing table fence
oesteban 5ec2bf6
fix: force-push a rewrite addressing minimal changes
oesteban a428a91
sty: make linter happy
oesteban 542ad99
Update src/05-derivatives/02-common-data-types.md
oesteban 9e4e384
Address @effigies' and @edickie's comments
oesteban c3213e6
sty: fix missaligned table fence
oesteban 9cdeea8
fix: better description for example following @edickie's comment
oesteban b071ebb
More generalizable concepts (addressing @robertoostenveld's comments)
oesteban ea3183a
Update src/05-derivatives/02-common-data-types.md
oesteban 6f21533
Address @robertoostenveld's comment
oesteban 68c9981
Merge branch 'enh/resolution-density' of github.com:oesteban/bids-spe…
oesteban 13b1d76
Merge remote-tracking branch 'upstream/common-derivatives' into enh/r…
oesteban 46d577c
fix: ``res-<index>`` -> ``res-<label>`` + admonitions
oesteban c8dd017
fix: cover one @effigies' suggestion
oesteban 7df0899
remove unnecessary edit
oesteban 0bbef9d
fix: example with bad formatting
oesteban 501c889
Merge remote-tracking branch 'upstream/common-derivatives' into enh/r…
effigies c56fad6
RF: Move res/den keywords to imaging derivatives
effigies 785c8cd
REVERT Removal of example from Common Data Types
effigies d484416
Drop tip that does not match context
effigies 8f834c7
STY: Satisfy linter
effigies 591f0a5
Merge remote-tracking branch 'upstream/common-derivatives' into enh/r…
oesteban 4002ab4
enh: include @robertoostenveld's suggestion
oesteban 0b99b7c
ENH: Add SpatialReference dictionary option and example
effigies bb94c0b
Merge remote-tracking branch 'upstream/common-derivatives' into enh/r…
effigies f60de90
STY: Drop parens
effigies e7f6341
Allow resolution/density to be a string or dictionary
effigies b874e08
Eliminate dtseries.json example, add space-fsLR to avoid ambiguity
effigies d518933
Update src/05-derivatives/01-introduction.md
effigies File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we stick with constant names from CIFTI-2 for volume as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm ok with changing to
VOLUME
but @oesteban where in cifti-2 did you find that constant?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just in case it carries other semantics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps I can explain my reasoning:
CORTEX_LEFT
,CORTEX_RIGHT
and a bunch of volumetric ones.CIFTI_STRUCTURE_ALL_GREY_MATTER
andCIFTI_STRUCTURE_OTHER_GREY_MATTER
.I'm not deeply opposed to VOLUME, but unlike the others, using doesn't reduce the impedance mismatch for CIFTI-2. So I'd lean towards BIDS-like conventions.