-
Notifications
You must be signed in to change notification settings - Fork 52
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: Support combined surface-volume reference spaces #883
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #883 +/- ##
==========================================
- Coverage 71.45% 71.43% -0.03%
==========================================
Files 87 87
Lines 8400 8443 +43
Branches 990 1010 +20
==========================================
+ Hits 6002 6031 +29
- Misses 2390 2404 +14
Partials 8 8 ☔ View full report in Codecov by Sentry. |
if self.space.startswith("fs"): | ||
object.__setattr__(self, "dim", 2) | ||
|
||
if "volspace" in self.spec: |
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.
This check would hopefully only be necessary in the short-term, since mixed standard/non-standard CIFTIs should be allowed.
cifti = attr.ib(default=False, repr=False, type=bool) | ||
"""Whether this space is a CIFTI space or not.""" |
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.
Alternatively, I could use a new dim
value, like 2.5, instead of adding this attribute.
Closes #881. This is a ways away from being usable, but it might be good to get folks' thoughts on it before I put too much effort into it.
The basic idea is that users can request any combination of surface and volume that they might want as output CIFTIs.
For example, the common
fsLR
space withden-91k
corresponds to CIFTIs with the left and right hemisphere surface components in fsLR space with 32k vertices in each hemisphere and the subcortical volumetric component in MNI152NLin6Asym space with 2 mm voxels. In my proposal, users would define that as--output-spaces fsLR:den-32k::MNI152NLin6Asym:res-02
. This could also be extended to other combinations, such as:To discuss:
<source_entities>_space-dhcpAsym_cohort-42_volspace-MNIInfant_volcohort-2_den-32k_res-1_bold.dtseries.nii
, maybe<source_entities>_space-dhcpAsym42+MNIInfant2_den-32k_res-1_bold.dtseries.nii
.cohort
,volspace
, andvolcohort
are not part of BIDS at the moment, so this approach would require fewer changes to BIDS (though we'd still need to allow+
symbols in entity values).Changes proposed:
::
) to allow combinations of surfaces and volumes for CIFTIs.spec
dictionary.cifti
attribute to the Reference object to denote if a space is CIFTI or not.