From 251362efba81be01e9e3b82d8bf6389fedaea7c5 Mon Sep 17 00:00:00 2001 From: oesteban Date: Thu, 18 Jul 2019 21:29:55 -0700 Subject: [PATCH 1/4] MAINT: Refactor ``aggregate_outputs`` for readability --- nipype/interfaces/base/core.py | 59 +++++++++++++++++----------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/nipype/interfaces/base/core.py b/nipype/interfaces/base/core.py index 0011c925dd..34037d9dfd 100644 --- a/nipype/interfaces/base/core.py +++ b/nipype/interfaces/base/core.py @@ -204,10 +204,10 @@ def _check_requires(self, spec, name, value): ] if any(values) and isdefined(value): if len(values) > 1: - fmt = ("%s requires values for inputs %s because '%s' is set. " + fmt = ("%s requires values for inputs %s because '%s' is set. " "For a list of required inputs, see %s.help()") else: - fmt = ("%s requires a value for input %s because '%s' is set. " + fmt = ("%s requires a value for input %s because '%s' is set. " "For a list of required inputs, see %s.help()") msg = fmt % (self.__class__.__name__, ', '.join("'%s'" % req for req in spec.requires), @@ -450,34 +450,35 @@ def _list_outputs(self): return None def aggregate_outputs(self, runtime=None, needed_outputs=None): - """ Collate expected outputs and check for existence - """ - - predicted_outputs = self._list_outputs() - outputs = self._outputs() - if predicted_outputs: - _unavailable_outputs = [] - if outputs: - _unavailable_outputs = \ - self._check_version_requirements(self._outputs()) - for key, val in list(predicted_outputs.items()): - if needed_outputs and key not in needed_outputs: - continue - if key in _unavailable_outputs: - raise KeyError(('Output trait %s not available in version ' - '%s of interface %s. Please inform ' - 'developers.') % (key, self.version, - self.__class__.__name__)) - try: - setattr(outputs, key, val) - except TraitError as error: - if getattr(error, 'info', - 'default').startswith('an existing'): - msg = ("File/Directory '%s' not found for %s output " - "'%s'." % (val, self.__class__.__name__, key)) - raise FileNotFoundError(msg) - raise error + """Collate expected outputs and apply output traits validation.""" + outputs = self._outputs() # Generate an empty output spec object + predicted_outputs = self._list_outputs() # Predictions from _list_outputs + if not predicted_outputs: + return outputs + # Precalculate the list of output trait names that should be aggregated + aggregate_names = set(predicted_outputs.keys()) + if needed_outputs is not None: + aggregate_names = set(needed_outputs).intersection(aggregate_names) + + if aggregate_names: # Make sure outputs are compatible + _na_outputs = self._check_version_requirements(outputs) + na_names = aggregate_names.intersection(set(_na_outputs)) + if na_names: + raise TypeError("""\ +Output trait(s) %s not available in version %s of interface %s.\ +""" % (', '.join(na_names), self.version, self.__class__.__name__)) + + for key in aggregate_names: # Final aggregation + val = predicted_outputs[key] + try: + setattr(outputs, key, val) + except TraitError as error: + if 'an existing' in getattr(error, 'info', 'default'): + msg = "No such file or directory for output '%s' of a %s interface" % \ + (key, self.__class__.__name__) + raise FileNotFoundError(val, message=msg) + raise error return outputs @property From 0c6d121883a8896cc1caf148f9a99d1e5efbb083 Mon Sep 17 00:00:00 2001 From: oesteban Date: Fri, 19 Jul 2019 19:40:49 -0700 Subject: [PATCH 2/4] fix: failing test since raised exception has been changed --- nipype/interfaces/base/tests/test_core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nipype/interfaces/base/tests/test_core.py b/nipype/interfaces/base/tests/test_core.py index 265edc444f..3192e75f81 100644 --- a/nipype/interfaces/base/tests/test_core.py +++ b/nipype/interfaces/base/tests/test_core.py @@ -289,7 +289,7 @@ def _list_outputs(self): return {'foo': 1} obj = DerivedInterface1() - with pytest.raises(KeyError): + with pytest.raises(TypeError): obj.run() From 259e7bc1071a79fb41436d9a37790b6497853aa0 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Fri, 26 Jul 2019 09:16:59 -0700 Subject: [PATCH 3/4] Apply suggestions from code review Co-Authored-By: Chris Markiewicz --- nipype/interfaces/base/core.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nipype/interfaces/base/core.py b/nipype/interfaces/base/core.py index 34037d9dfd..279f9a81f0 100644 --- a/nipype/interfaces/base/core.py +++ b/nipype/interfaces/base/core.py @@ -457,13 +457,13 @@ def aggregate_outputs(self, runtime=None, needed_outputs=None): return outputs # Precalculate the list of output trait names that should be aggregated - aggregate_names = set(predicted_outputs.keys()) + aggregate_names = set(predicted_outputs) if needed_outputs is not None: aggregate_names = set(needed_outputs).intersection(aggregate_names) if aggregate_names: # Make sure outputs are compatible _na_outputs = self._check_version_requirements(outputs) - na_names = aggregate_names.intersection(set(_na_outputs)) + na_names = aggregate_names.intersection(_na_outputs) if na_names: raise TypeError("""\ Output trait(s) %s not available in version %s of interface %s.\ From bb736ae60e08f6dbf9e60e14871fdd995a744e7e Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Fri, 26 Jul 2019 10:13:37 -0700 Subject: [PATCH 4/4] Apply suggestions from code review Co-Authored-By: Chris Markiewicz --- nipype/interfaces/base/core.py | 3 ++- nipype/interfaces/base/tests/test_core.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/nipype/interfaces/base/core.py b/nipype/interfaces/base/core.py index 279f9a81f0..9345ef731a 100644 --- a/nipype/interfaces/base/core.py +++ b/nipype/interfaces/base/core.py @@ -465,7 +465,8 @@ def aggregate_outputs(self, runtime=None, needed_outputs=None): _na_outputs = self._check_version_requirements(outputs) na_names = aggregate_names.intersection(_na_outputs) if na_names: - raise TypeError("""\ + # XXX Change to TypeError in Nipype 2.0 + raise KeyError("""\ Output trait(s) %s not available in version %s of interface %s.\ """ % (', '.join(na_names), self.version, self.__class__.__name__)) diff --git a/nipype/interfaces/base/tests/test_core.py b/nipype/interfaces/base/tests/test_core.py index 3192e75f81..265edc444f 100644 --- a/nipype/interfaces/base/tests/test_core.py +++ b/nipype/interfaces/base/tests/test_core.py @@ -289,7 +289,7 @@ def _list_outputs(self): return {'foo': 1} obj = DerivedInterface1() - with pytest.raises(TypeError): + with pytest.raises(KeyError): obj.run()