[ENH] Add volspace entity for CIFTI files

[tsalo]

This PR adds a volspace entity to describe the volumetric space in combined surface-volume derivative files (i.e., CIFTIs). The volspace entity comes from BEP011: Structural derivatives (#518).

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.10%. Comparing base (9377b20) to head (60248c3).
Report is 309 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1900   +/-   ##
=======================================
  Coverage   88.10%   88.10%           
=======================================
  Files          16       16           
  Lines        1396     1396           
=======================================
  Hits         1230     1230           
  Misses        166      166           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[effigies]

TBH I don't like it very much (see https://github.com/bids-standard/bids-specification/pull/518/files#r447942761). I would rather revive #1165 than have two entities with such similar purposes.

A space can refer to a dictionary of references (https://bids-specification.readthedocs.io/en/stable/derivatives/common-data-types.html#spatial-references), so you can already just make up a name and define it. Permitting + would just be a convenience to not have to come up with a name for a new combined space.

[tsalo]

I had hoped to use + for cohorts, but what would that look like with volspace as well? Something like dhcpAsym+42+MNIInfant+2 seems a little unwieldy.

[effigies]

I see, the concern is parsing filenames in order to look up associated files in something like templateflow. That makes me think that metadata is probably the better way to go. What if you had something like space-dhcpAsym42+MNIInfant2_<suffix>.json:

{
  "SpatialReference": {
    "VolumeReference": "bids:MNIInfant:cohort-2/tpl-MNIInfant_cohort-2_res-1_T2w.nii.gz",
    "CIFTI_STRUCTURE_CORTEX_LEFT": "bids:dhcpAsym:cohort-42/tpl-dhcpAsym_cohort-42_hemi-L_den-32k_white.surf.gii",
    "CIFTI_STRUCTURE_CORTEX_RIGHT": "bids:dhcpAsym:cohort-42/tpl-dhcpAsym_cohort-42_hemi-R_den-32k_white.surf.gii"
  }
}

(I'm making up these filenames. They may not be right.)

[tsalo]

According to the spec, space values must be defined in the schema's list of coordinate spaces. I imagine we could have something in the spec/schema about parsing the space values, though it would make the validator a bit more complicated I assume.

I'm fine switching to your proposal, but I'm a little worried about what the effects would be on downstream software (e.g., niworkflows, fMRIPrep, XCP-D). Adding new entities is straightforward, but changing how we structure space definitions might be tougher. Maybe I'm just not doing a good job of predicting the consequences.

[effigies]

According to the spec, space values must be defined in the schema's list of coordinate spaces.

Reference? AFAIK this was never intended to be exhaustive but to provide common names for frequently-used spaces.

[tsalo]

https://bids-specification.readthedocs.io/en/stable/appendices/entities.html#space

The <label> MUST be taken from one of the modality specific lists in the Coordinate Systems Appendix.

[effigies]

Got it, thanks. That requirement was introduced in #734, and it appears to be for the sake of validating raw iEEG data (https://github.com/bids-standard/bids-validator/issues/743) and has been enforced for iEEG and EEG (https://github.com/bids-standard/bids-validator/pull/1190) only.

It has been widely flouted in the derivatives context, at least. fMRIPrep, for example, has used spaces T1w, anat, boldref and func, to denote spaces with explicit images. Further, the schema rules are:

bids-specification/src/schema/rules/sidecars/derivatives/common_derivatives.yaml

Lines 12 to 28 in 7b0f76d

	SpatialReferenceEntity:
	selectors:
	- dataset.dataset_description.DatasetType == "derivative"
	- '"space" in entities'
	fields:
	SpatialReference:
	level: recommended
	level_addendum: |
	if the derivative is aligned to a standard template listed in
	[Standard template identifiers][templates]. Required otherwise.
	SpatialReferenceNonStandard:
	selectors:
	- dataset.dataset_description.DatasetType == "derivative"
	- '!intersects(schema.objects.metadata._StandardTemplateCoordSys, [entities.space])'
	fields:
	SpatialReference: required

This apparently doesn't have corresponding text, but it is what the schema validator is enforcing. The text I would write would be:

If the space-<label> entity is not defined in the BIDS specification (Coordinate Systems Appendix),
then the SpatialReference metadata field MUST be defined.

[tsalo]

To summarize what I think the current state of the conversation is, we are going to try to support +s in entity labels (see #1926), and surface space, surface cohort, volume space, and volume cohort will all be encoded in the space entity.

Additionally, in some cases, we will use surface template names as shorthand for specific surface-volume combinations. For example, space-fsLR in a CIFTI means that the surface is fsLR and the volume is MNI152NLin6Asym.

I don't want to extend that to every surface-volume combination though. For example, if the onavg surface template folks prefer to combine it with MNI152NLin2009cAsym, I'd like to see that labeled as space-onavg+MNI152NLin2009cAsym rather than just space-onavg, but I guess we can discuss that elsewhere.

In any case, I guess I can close this in favor of #1926.