Skip to content

[MAINT] Outsource get_filecopy_info() from interfaces #2798

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

Merged
merged 2 commits into from
Nov 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 9 additions & 17 deletions nipype/interfaces/base/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
from .specs import (BaseInterfaceInputSpec, CommandLineInputSpec,
StdOutCommandLineInputSpec, MpiCommandLineInputSpec)
from .support import (Bunch, InterfaceResult, NipypeInterfaceError)
from .specs import get_filecopy_info

from future import standard_library
standard_library.install_aliases()
Expand Down Expand Up @@ -122,11 +123,15 @@ def _list_outputs(self):
""" List expected outputs"""
raise NotImplementedError

def _get_filecopy_info(self):
""" Provides information about file inputs to copy or link to cwd.
Necessary for pipeline operation
@classmethod
def _get_filecopy_info(cls):
"""Provides information about file inputs to copy or link to cwd.
Necessary for pipeline operation
"""
raise NotImplementedError
iflogger.warning(
'_get_filecopy_info member of Interface was deprecated '
'in nipype-1.1.6 and will be removed in 1.2.0')
return get_filecopy_info(cls)


class BaseInterface(Interface):
Expand Down Expand Up @@ -330,19 +335,6 @@ def _outputs(self):

return outputs

@classmethod
def _get_filecopy_info(cls):
""" Provides information about file inputs to copy or link to cwd.
Necessary for pipeline operation
"""
info = []
if cls.input_spec is None:
return info
metadata = dict(copyfile=lambda t: t is not None)
for name, spec in sorted(cls.input_spec().traits(**metadata).items()):
info.append(dict(key=name, copy=spec.copyfile))
return info

def _check_requires(self, spec, name, value):
""" check if required inputs are satisfied
"""
Expand Down
19 changes: 19 additions & 0 deletions nipype/interfaces/base/specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
absolute_import)

import os
from inspect import isclass
from copy import deepcopy
from warnings import warn
from builtins import str, bytes
Expand Down Expand Up @@ -375,3 +376,21 @@ class MpiCommandLineInputSpec(CommandLineInputSpec):
n_procs = traits.Int(desc="Num processors to specify to mpiexec. Do not "
"specify if this is managed externally (e.g. through "
"SGE)")


def get_filecopy_info(cls):
"""Provides information about file inputs to copy or link to cwd.
Necessary for pipeline operation
"""
if cls.input_spec is None:
return None

# normalize_filenames is not a classmethod, hence check first
if not isclass(cls) and hasattr(cls, 'normalize_filenames'):
cls.normalize_filenames()
info = []
inputs = cls.input_spec() if isclass(cls) else cls.inputs
metadata = dict(copyfile=lambda t: t is not None)
for name, spec in sorted(inputs.traits(**metadata).items()):
info.append(dict(key=name, copy=spec.copyfile))
return info
7 changes: 0 additions & 7 deletions nipype/interfaces/base/tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,12 @@ def __init__(self):
nif.aggregate_outputs()
with pytest.raises(NotImplementedError):
nif._list_outputs()
with pytest.raises(NotImplementedError):
nif._get_filecopy_info()


def test_BaseInterface():
config.set('monitoring', 'enable', '0')

assert nib.BaseInterface.help() is None
assert nib.BaseInterface._get_filecopy_info() == []

class InputSpec(nib.TraitedSpec):
foo = nib.traits.Int(desc='a random int')
Expand All @@ -90,10 +87,6 @@ class DerivedInterface(nib.BaseInterface):
assert DerivedInterface.help() is None
assert 'moo' in ''.join(DerivedInterface._inputs_help())
assert DerivedInterface()._outputs() is None
assert DerivedInterface._get_filecopy_info()[0]['key'] == 'woo'
assert DerivedInterface._get_filecopy_info()[0]['copy']
assert DerivedInterface._get_filecopy_info()[1]['key'] == 'zoo'
assert not DerivedInterface._get_filecopy_info()[1]['copy']
assert DerivedInterface().inputs.foo == nib.Undefined
with pytest.raises(ValueError):
DerivedInterface()._check_mandatory_inputs()
Expand Down
48 changes: 45 additions & 3 deletions nipype/interfaces/base/tests/test_specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from ....interfaces import fsl
from ...utility.wrappers import Function
from ....pipeline import Node

from ..specs import get_filecopy_info

standard_library.install_aliases()

Expand Down Expand Up @@ -55,8 +55,8 @@ def test_TraitedSpec_tab_completion():
bet_nd = Node(fsl.BET(), name='bet')
bet_interface = fsl.BET()
bet_inputs = bet_nd.inputs.class_editable_traits()
bet_outputs = bet_nd.outputs.class_editable_traits()
bet_outputs = bet_nd.outputs.class_editable_traits()

# Check __all__ for bet node and interface inputs
assert set(bet_nd.inputs.__all__) == set(bet_inputs)
assert set(bet_interface.inputs.__all__) == set(bet_inputs)
Expand Down Expand Up @@ -433,3 +433,45 @@ def test_ImageFile():
with pytest.raises(nib.TraitError):
x.nocompress = 'test.nii.gz'
x.nocompress = 'test.mgh'


def test_filecopy_info():
class InputSpec(nib.TraitedSpec):
foo = nib.traits.Int(desc='a random int')
goo = nib.traits.Int(desc='a random int', mandatory=True)
moo = nib.traits.Int(desc='a random int', mandatory=False)
hoo = nib.traits.Int(desc='a random int', usedefault=True)
zoo = nib.File(desc='a file', copyfile=False)
woo = nib.File(desc='a file', copyfile=True)

class DerivedInterface(nib.BaseInterface):
input_spec = InputSpec
resource_monitor = False

def normalize_filenames(self):
"""A mock normalize_filenames for freesurfer interfaces that have one"""
self.inputs.zoo = 'normalized_filename.ext'

assert get_filecopy_info(nib.BaseInterface) == []

# Test on interface class, not instantiated
info = get_filecopy_info(DerivedInterface)
assert info[0]['key'] == 'woo'
assert info[0]['copy']
assert info[1]['key'] == 'zoo'
assert not info[1]['copy']
info = None

# Test with instantiated interface
derived = DerivedInterface()
# First check that zoo is not defined
assert derived.inputs.zoo == Undefined
# After the first call to get_filecopy_info zoo is defined
info = get_filecopy_info(derived)
# Ensure that normalize_filenames was called
assert derived.inputs.zoo == 'normalized_filename.ext'
# Check the results are consistent
assert info[0]['key'] == 'woo'
assert info[0]['copy']
assert info[1]['key'] == 'zoo'
assert not info[1]['copy']
10 changes: 0 additions & 10 deletions nipype/interfaces/freesurfer/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,16 +189,6 @@ class FSSurfaceCommand(FSCommand):
including the full path in the filename, we can also avoid this behavior.
"""

def _get_filecopy_info(self):
self._normalize_filenames()
return super(FSSurfaceCommand, self)._get_filecopy_info()

def _normalize_filenames(self):
"""Filename normalization routine to perform only when run in Node
context
"""
pass

@staticmethod
def _associated_file(in_file, out_name):
"""Based on MRIsBuildFileName in freesurfer/utils/mrisurf.c
Expand Down
15 changes: 10 additions & 5 deletions nipype/interfaces/freesurfer/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1289,9 +1289,11 @@ def _list_outputs(self):

return outputs

def _normalize_filenames(self):
""" In a Node context, interpret out_file as a literal path to
reduce surprise.
def normalize_filenames(self):
"""
Filename normalization routine to perform only when run in Node
context.
Interpret out_file as a literal path to reduce surprise.
"""
if isdefined(self.inputs.out_file):
self.inputs.out_file = os.path.abspath(self.inputs.out_file)
Expand Down Expand Up @@ -3837,8 +3839,11 @@ def _list_outputs(self):
self.inputs.out_name)
return outputs

def _normalize_filenames(self):
""" Find full paths for pial, thickness and sphere files for copying
def normalize_filenames(self):
"""
Filename normalization routine to perform only when run in Node
context.
Find full paths for pial, thickness and sphere files for copying.
"""
in_file = self.inputs.in_file

Expand Down
7 changes: 5 additions & 2 deletions nipype/pipeline/engine/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
from ...interfaces.base import (traits, InputMultiPath, CommandLine, Undefined,
DynamicTraitedSpec, Bunch, InterfaceResult,
Interface, isdefined)
from ...interfaces.base.specs import get_filecopy_info

from .utils import (
_parameterization_dir, save_hashfile as _save_hashfile, load_resultfile as
_load_resultfile, save_resultfile as _save_resultfile, nodelist_runner as
Expand Down Expand Up @@ -656,7 +658,8 @@ def _run_command(self, execute, copyfiles=True):

def _copyfiles_to_wd(self, execute=True, linksonly=False):
"""copy files over and change the inputs"""
if not hasattr(self._interface, '_get_filecopy_info'):
filecopy_info = get_filecopy_info(self.interface)
if not filecopy_info:
# Nothing to be done
return

Expand All @@ -669,7 +672,7 @@ def _copyfiles_to_wd(self, execute=True, linksonly=False):
outdir = op.join(outdir, '_tempinput')
makedirs(outdir, exist_ok=True)

for info in self._interface._get_filecopy_info():
for info in filecopy_info:
files = self.inputs.trait_get().get(info['key'])
if not isdefined(files) or not files:
continue
Expand Down