Skip to content

Adding niftyreg #1913

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 20 commits into from
Apr 28, 2017
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
a97b291
Initial commit adding the interfaces to the niftyreg executables
mmodat Mar 28, 2017
62134cf
Modifications towards python3
mmodat Mar 29, 2017
44d01b6
Fixing bytes data to string for comparaison for python 3 compatibility
byvernault Mar 29, 2017
37411dd
Editing the workflow to be compatible python 2.7/3.x
byvernault Mar 29, 2017
9e75456
Edit version control
byvernault Mar 29, 2017
b9e37e4
Skipping some test in docstring
byvernault Mar 30, 2017
403947b
Adding # doctest: +SKIP
byvernault Mar 30, 2017
33ebab8
Editing docstring to follow nipype template and add some tests.
byvernault Mar 31, 2017
6800cce
Removing code to raise exceptions for version control and switch to w…
byvernault Apr 3, 2017
cc1c6d0
Editing the tests and docstring
byvernault Apr 3, 2017
18994c6
Adding # doctest: +ALLOW_UNICODE for unicode
byvernault Apr 3, 2017
1ed121d
using files present in testing/data
byvernault Apr 3, 2017
81b738b
moving print to warning and editing docstring
byvernault Apr 4, 2017
4171b19
fixing the last docstring
byvernault Apr 4, 2017
f12762a
Symplifying interfaces using name_source/name_template when possible
byvernault Apr 20, 2017
1527855
Seeting omp to 1 if not set and removing PositiveInt to use traits.Range
byvernault Apr 21, 2017
b84f217
Adding NiftyRegCommandInputSpec and using overload_extension
byvernault Apr 24, 2017
319d6e3
Edit for changes suggested by oesteban
byvernault Apr 26, 2017
61bfeb4
fixing flake8 issue
byvernault Apr 27, 2017
215f5f0
setting self.numthreads to omp_core_val if set
byvernault Apr 28, 2017
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
15 changes: 15 additions & 0 deletions nipype/interfaces/niftyreg/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# -*- coding: utf-8 -*-
# emacs: -*- mode: python; py-indent-offset: 4; indent-tabs-mode: nil -*-
# vi: set ft=python sts=4 ts=4 sw=4 et:

"""
The niftyreg module provides classes for interfacing with the `NiftyReg
<http://sourceforge.net/projects/niftyreg/>`_ command line tools.

Top-level namespace for niftyreg.
"""

from .base import no_niftyreg, get_custom_path, PositiveInt
from .reg import RegAladin, RegF3D
from .regutils import (RegResample, RegJacobian, RegAverage, RegTools,
RegTransform, RegMeasure)
131 changes: 131 additions & 0 deletions nipype/interfaces/niftyreg/base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
# -*- coding: utf-8 -*-
# emacs: -*- mode: python; py-indent-offset: 4; indent-tabs-mode: nil -*-
# vi: set ft=python sts=4 ts=4 sw=4 et:

"""
The niftyreg module provides classes for interfacing with `niftyreg
<http://sourceforge.net/projects/niftyreg/>`_ command line tools.

These are the base tools for working with niftyreg.

Registration tools are found in niftyreg/reg.py
Every other tool is found in niftyreg/regutils.py

Examples
--------
See the docstrings of the individual classes for examples.

"""

from __future__ import (print_function, division, unicode_literals,
absolute_import)
from builtins import property, super
from distutils.version import StrictVersion
import os
import subprocess
import warnings

from ..base import CommandLine, isdefined, traits
from ...utils.filemanip import split_filename


warn = warnings.warn
warnings.filterwarnings('always', category=UserWarning)


def get_custom_path(command):
try:
specific_dir = os.environ['NIFTYREGDIR']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about:

def get_custom_path(command):
    return os.path.join(os.getenv('NIFTYREGDIR', ''), command)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I should have changed this before to what you have. Thx. I will edit it.

command = os.path.join(specific_dir, command)
return command
except KeyError:
return command


def no_niftyreg(cmd='reg_f3d'):
if True in [os.path.isfile(os.path.join(path, cmd)) and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True in [...] should be replaced by any:

import shutil
def no_niftyreg(cmd='reg_f3d'):
    try:
        return shutil.which(cmd) is None
    except AttributeError:  # Python < 3.3
        return not any([os.path.isfile(os.path.join(path, cmd)) and os.access(os.path.join(path, cmd), os.X_OK) for path in os.environ["PATH"].split(os.pathsep)]

@satra, I think we should provide an all-python compatible which function in filemanip. WDYT?

Copy link
Contributor Author

@byvernault byvernault Apr 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it makes sense.

os.access(os.path.join(path, cmd), os.X_OK)
for path in os.environ["PATH"].split(os.pathsep)]:
return False
return True


class PositiveInt(traits.BaseInt):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while this works, you could have used: a = traits.Range(low=0)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@satra, thx. I just changed all of them to use Range traits instead of rewritting something that does the same.

Cheers,

# Define the default value
default_value = 0
# Describe the trait type
info_text = 'A positive integer'

def validate(self, obj, name, value):
value = super(PositiveInt, self).validate(obj, name, value)
if (value >= 0) == 1:
return value
self.error(obj, name, value)


class NiftyRegCommand(CommandLine):
"""
Base support interface for NiftyReg commands.
"""
_suffix = '_nr'
_min_version = '1.5.10'

def __init__(self, required_version=None, **inputs):
super(NiftyRegCommand, self).__init__(**inputs)
cur_version = self.get_version()
if not cur_version:
raise Exception('Niftyreg not found')
# Decoding to string:
cur_version = cur_version.decode("utf-8")
if StrictVersion(cur_version) < StrictVersion(self._min_version):
err = 'A later version of Niftyreg is required (%s < %s)'
raise ValueError(err % (cur_version, self._min_version))
if required_version is not None:
if StrictVersion(cur_version) != StrictVersion(required_version):
err = 'The version of NiftyReg differs from the required'
err += '(%s != %s)'
raise ValueError(err % (cur_version, required_version))

def get_version(self):
if no_niftyreg(cmd=self.cmd):
return None
exec_cmd = ''.join((self.cmd, ' -v'))
return subprocess.check_output(exec_cmd, shell=True).strip()

@property
def version(self):
return self.get_version()

def exists(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

def exists(self):
    return not self.get_version() is None

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to butt in, but not x is None should be x is not None. Just a style issue, but a lot of style checkers will yell at you.

Speaking of which, have you run all of this through a style checker, such as flake8? Nipype has somewhat intermittent style compliance, but for new contributions it's good to start off on the right foot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, no problem.

I am using flake8 but it does not complain with this one. I will edit it.

Cheers,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about that

if self.get_version() is None:
return False
return True

def _gen_fname(self, basename, out_dir=None, suffix=None, ext=None):
if basename == '':
msg = 'Unable to generate filename for command %s. ' % self.cmd
msg += 'basename is not set!'
raise ValueError(msg)
_, final_bn, final_ext = split_filename(basename)
if out_dir is None:
out_dir = os.getcwd()
if ext is not None:
final_ext = ext
if suffix is not None:
final_bn = ''.join((final_bn, suffix))
return os.path.abspath(os.path.join(out_dir, final_bn + final_ext))

def _gen_filename(self, name):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function, the previous one, and the next one can be removed and the traits extended with name_source and name_template. i'll provide an example when i come across the relevant interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @satra , it's used in the interfaces for RegTools for example to avoid repeating some of the code. Should I remove them and put them in each interfaces?

In those interfaces, we name the output depending on the input image and the type specified. Can we use two values with name_source and name_template?

Cheers,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it depends on what you would like to do. there are two things that may help:

  1. keep_extension (traits metadata): allows carrying the extension of name_source through to output.

  2. _overload_extension (class function): here is an example of use in the afni base class. https://github.com/nipy/nipype/blob/master/nipype/interfaces/afni/base.py#L218

Copy link
Contributor Author

@byvernault byvernault Apr 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@satra, on this part of the interfaces, I left it as it is since for some interfaces, we can not use the name_source since it doesn't allow naming with several variables if I understood the doc well.

For example, Reg_Measure, set the outfile with the basename flo_file where we add the measure_type as suffix and then the extension.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah i see. while this can indeed be done in overload_extension, we will likely have a cleaner mechanism in the next release to support multiple variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to know.

Do you want us to use the overload_extension in all those cases or leave it like that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would prefer overload extension over gen_filename at this point. when we implement the multiple variables option, it will simply allow us to improve the namesource and template.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, working on it.

if name == 'out_file':
return self._gen_fname(self.inputs.in_file,
suffix=self._suffix,
ext='.nii.gz')
return None

def _list_outputs(self):
outputs = self.output_spec().get()
if isdefined(self.inputs.out_file):
outputs['out_file'] = self.inputs.out_file
else:
outputs['out_file'] = self._gen_filename('out_file')
return outputs
Loading