From e0f04ecb6273191cc0bbfb021a00d82d7d5d44fe Mon Sep 17 00:00:00 2001 From: mathiasg Date: Fri, 3 Apr 2020 15:53:58 -0400 Subject: [PATCH 1/5] ENH: Make default native resolution explicit --- niworkflows/utils/spaces.py | 11 ++++++---- niworkflows/utils/tests/test_spaces.py | 28 ++++++++++++++++++-------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/niworkflows/utils/spaces.py b/niworkflows/utils/spaces.py index cf3c9a7d540..3c9a89e24cb 100644 --- a/niworkflows/utils/spaces.py +++ b/niworkflows/utils/spaces.py @@ -660,10 +660,13 @@ 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 ( + not val.split(':')[0].startswith('fs') + and ":res-" not in val + and ":resolution-" not in val + ): + # by default, explicitly set volumetric resolution to native + 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 index d247a2e8cba..627c3cdc322 100644 --- a/niworkflows/utils/tests/test_spaces.py +++ b/niworkflows/utils/tests/test_spaces.py @@ -17,14 +17,24 @@ 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" + )) ]) def test_space_action(parser, spaces, expected): """Test action.""" @@ -33,7 +43,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)]) From a3edc3de3e0e6e8882401c76e9318eec0962ec51 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Sun, 5 Apr 2020 23:43:33 -0400 Subject: [PATCH 2/5] enh: strip native resolution identifier from derivatives --- niworkflows/interfaces/bids.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/niworkflows/interfaces/bids.py b/niworkflows/interfaces/bids.py index ab6c965bdd8..6adcb1fe0e8 100644 --- a/niworkflows/interfaces/bids.py +++ b/niworkflows/interfaces/bids.py @@ -454,6 +454,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 '' From 73f4dc1c73194730fb31122dfc6192df817badc6 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Sun, 5 Apr 2020 23:50:45 -0400 Subject: [PATCH 3/5] ENH: avoid setting nonstandard references resolutions --- niworkflows/utils/spaces.py | 3 ++- niworkflows/utils/tests/test_spaces.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/niworkflows/utils/spaces.py b/niworkflows/utils/spaces.py index 3c9a89e24cb..914683e0319 100644 --- a/niworkflows/utils/spaces.py +++ b/niworkflows/utils/spaces.py @@ -661,7 +661,8 @@ def __call__(self, parser, namespace, values, option_string=None): for val in values: val = val.rstrip(":") if ( - not val.split(':')[0].startswith('fs') + val not in NONSTANDARD_REFERENCES + and not val.split(':')[0].startswith('fs') and ":res-" not in val and ":resolution-" not in val ): diff --git a/niworkflows/utils/tests/test_spaces.py b/niworkflows/utils/tests/test_spaces.py index 627c3cdc322..a9e7a3eeeeb 100644 --- a/niworkflows/utils/tests/test_spaces.py +++ b/niworkflows/utils/tests/test_spaces.py @@ -34,7 +34,8 @@ def parser(): "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.""" From d830e55cc2c4cfe07c34e83a67732f927d822547 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Mon, 6 Apr 2020 13:16:40 -0400 Subject: [PATCH 4/5] DOC: add space usage to docstring --- niworkflows/interfaces/bids.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/niworkflows/interfaces/bids.py b/niworkflows/interfaces/bids.py index 6adcb1fe0e8..70809bbe398 100644 --- 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] From f59e306ec177a48ed6263e9d1df7dd6f137a3c0a Mon Sep 17 00:00:00 2001 From: mathiasg Date: Tue, 7 Apr 2020 13:14:24 -0400 Subject: [PATCH 5/5] doc: insert urls to discussions [skip ci] --- niworkflows/utils/spaces.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/niworkflows/utils/spaces.py b/niworkflows/utils/spaces.py index 914683e0319..0305e57d061 100644 --- a/niworkflows/utils/spaces.py +++ b/niworkflows/utils/spaces.py @@ -667,6 +667,9 @@ def __call__(self, parser, namespace, values, option_string=None): 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)