Merge pull request #494 from mgxd/fix/default-refs-native

ENH: Make default native resolution explicit
nipreps · Apr 7, 2020 · d109e99 · d109e99
2 parents c9ec849 + f59e306
commit d109e99
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 14 deletions.
diff --git a/niworkflows/interfaces/bids.py b/niworkflows/interfaces/bids.py
@@ -342,11 +342,12 @@ class DerivativesDataSink(SimpleInterface):
     >>> dsink.inputs.source_file = str(tricky_source)
     >>> dsink.inputs.keep_dtype = True
     >>> dsink.inputs.desc = 'preproc'
+    >>> dsink.inputs.space = 'MNI152NLin6Asym_res-01'
     >>> dsink.inputs.RepetitionTime = 0.75
     >>> res = dsink.run()
     >>> res.outputs.out_meta  # doctest: +ELLIPSIS
     '.../niworkflows/sub-02/ses-noanat/func/sub-02_ses-noanat_task-rest_run-01_\
-desc-preproc_bold.json'
+space-MNI152NLin6Asym_res-01_desc-preproc_bold.json'
 
     >>> lines = Path(res.outputs.out_meta).read_text().splitlines()
     >>> lines[1]
@@ -365,12 +366,13 @@ class DerivativesDataSink(SimpleInterface):
     >>> dsink.inputs.source_file = str(tricky_source)
     >>> dsink.inputs.keep_dtype = True
     >>> dsink.inputs.desc = 'preproc'
+    >>> dsink.inputs.space = 'MNI152NLin6Asym_res-native'
     >>> dsink.inputs.RepetitionTime = 0.75
     >>> dsink.inputs.meta_dict = {'RepetitionTime': 1.75, 'SkullStripped': False, 'Z': 'val'}
     >>> res = dsink.run()
     >>> res.outputs.out_meta  # doctest: +ELLIPSIS
     '.../niworkflows/sub-02/ses-noanat/func/sub-02_ses-noanat_task-rest_run-01_\
-desc-preproc_bold.json'
+space-MNI152NLin6Asym_desc-preproc_bold.json'
 
     >>> lines = Path(res.outputs.out_meta).read_text().splitlines()
     >>> lines[1]
@@ -454,6 +456,10 @@ def _run_interface(self, runtime):
         if len(self.inputs.in_file) > 1 and not isdefined(self.inputs.extra_values):
             formatstr = formatbase + '{suffix}{i:04d}{dtype}{ext}'
 
+        if self.inputs.space and '_res-native' in self.inputs.space:
+            # strip native tag
+            self.inputs.space = self.inputs.space.split('_')[0]
+
         space = '_space-{}'.format(self.inputs.space) if self.inputs.space else ''
         desc = '_desc-{}'.format(self.inputs.desc) if self.inputs.desc else ''
         suffix = '_{}'.format(self.inputs.suffix) if self.inputs.suffix else ''

diff --git a/niworkflows/utils/spaces.py b/niworkflows/utils/spaces.py
@@ -660,10 +660,17 @@ def __call__(self, parser, namespace, values, option_string=None):
             spaces.checkpoint()
         for val in values:
             val = val.rstrip(":")
-            # Should we support some sort of explicit "default" resolution?
-            # https://github.com/nipreps/niworkflows/pull/457#discussion_r375510227
-            # if ":res-" not in val or ":resolution-" not in val:
-            #     val = ":".join((val, "res-default"))
+            if (
+                val not in NONSTANDARD_REFERENCES
+                and not val.split(':')[0].startswith('fs')
+                and ":res-" not in val
+                and ":resolution-" not in val
+            ):
+                # by default, explicitly set volumetric resolution to native
+                # relevant discussions:
+                # https://github.com/nipreps/niworkflows/pull/457#discussion_r375510227
+                # https://github.com/nipreps/niworkflows/pull/494
+                val = ":".join((val, "res-native"))
             for sp in Reference.from_string(val):
                 spaces.add(sp)
         setattr(namespace, self.dest, spaces)

diff --git a/niworkflows/utils/tests/test_spaces.py b/niworkflows/utils/tests/test_spaces.py
@@ -17,14 +17,25 @@ def parser():
     return pars
 
 
-@pytest.mark.parametrize("spaces,expected", [
-    (("MNI152NLin6Asym",), 1),
-    (("fsaverage:den-10k", "MNI152NLin6Asym"), 2),
-    (("fsaverage:den-10k:den-30k", "MNI152NLin6Asym:res-1:res-2"), 4),
+@pytest.mark.parametrize("spaces, expected", [
+    (("MNI152NLin6Asym",), ("MNI152NLin6Asym:res-native",)),
+    (("fsaverage:den-10k", "MNI152NLin6Asym"), (
+        "fsaverage:den-10k", "MNI152NLin6Asym:res-native"
+    )),
+    (("fsaverage:den-10k:den-30k", "MNI152NLin6Asym:res-1:res-2"), (
+        "fsaverage:den-10k", "fsaverage:den-30k",
+        "MNI152NLin6Asym:res-1", "MNI152NLin6Asym:res-2"
+    )),
+    (("fsaverage:den-10k:den-30k", "MNI152NLin6Asym:res-1:res-2", "fsaverage5"), (
+        "fsaverage:den-10k", "fsaverage:den-30k",
+        "MNI152NLin6Asym:res-1", "MNI152NLin6Asym:res-2"
+    )),
     (("fsaverage:den-10k:den-30k", "MNI152NLin6Asym:res-1:res-2",
-      "fsaverage5"), 4),
-    (("fsaverage:den-10k:den-30k", "MNI152NLin6Asym:res-1:res-2",
-      "fsaverage:den-10k:den-30k", "MNI152NLin6Asym:res-1:res-2"), 4),
+      "fsaverage:den-10k:den-30k", "MNI152NLin6Asym:res-1:res-2"), (
+        "fsaverage:den-10k", "fsaverage:den-30k",
+        "MNI152NLin6Asym:res-1", "MNI152NLin6Asym:res-2"
+    )),
+    (("MNI152NLin6Asym", "func"), ("MNI152NLin6Asym:res-native", "func"))
 ])
 def test_space_action(parser, spaces, expected):
     """Test action."""
@@ -33,7 +44,9 @@ def test_space_action(parser, spaces, expected):
     assert isinstance(parsed_spaces, SpatialReferences)
     assert all(isinstance(sp, Reference) for sp in parsed_spaces.references), \
         "Every element must be a `Reference`"
-    assert len(parsed_spaces.references) == expected
+    assert len(parsed_spaces.references) == len(expected)
+    for ref, expected_ref in zip(parsed_spaces.references, expected):
+        assert str(ref) == expected_ref
 
 
 @pytest.mark.parametrize("flag,expected", [(('--spaces',), True), (None, False)])