enh: trait for imaging files + implementation in SPM preproc #1949

mgxd · 2017-04-11T19:41:46Z

should be extended to the other spm interfaces - is there a better way to approach this?

also updated some missed auto-tests

codecov-io · 2017-04-11T21:32:28Z

Codecov Report

Merging #1949 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #1949      +/-   ##
=========================================
+ Coverage   72.29%   72.3%   +0.01%     
=========================================
  Files        1089    1089              
  Lines       55586   55626      +40     
  Branches     8010    8017       +7     
=========================================
+ Hits        40184   40223      +39     
- Misses      14148   14149       +1     
  Partials     1254    1254

Flag	Coverage Δ
#smoketests	`72.3% <100%> (+0.01%)`	⬆️
#unittests	`69.83% <100%> (+0.02%)`	⬆️

Impacted Files	Coverage Δ
nipype/interfaces/base.py	`84.66% <ø> (ø)`	⬆️
nipype/interfaces/fsl/tests/test_auto_TOPUP.py	`85.71% <ø> (ø)`	⬆️
nipype/interfaces/fsl/tests/test_auto_Eddy.py	`85.71% <ø> (ø)`	⬆️
nipype/interfaces/spm/preprocess.py	`68.93% <100%> (ø)`	⬆️
nipype/interfaces/traits_extension.py	`96.87% <100%> (-0.39%)`	⬇️
nipype/interfaces/spm/base.py	`84.69% <100%> (+0.27%)`	⬆️
nipype/interfaces/tests/test_base.py	`96.74% <100%> (+0.07%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae7af2d...bf564f7. Read the comment docs.

effigies · 2017-04-14T18:37:37Z

nipype/interfaces/spm/preprocess.py

+                        raise TypeError('Input files must be uncompressed')
+            else:
+                if is_gz(self.inputs.in_files):
+                    raise TypeError('Input files must be uncompressed')


Not sure if there's a better approach in general, but the test can be made much more concise:

if isdefined(self.inputs.in_files) and any(imgf.endswith('.gz') for imgf in filename_to_list(self.inputs.in_files)): raise TypeError('Input files must be uncompressed')

satra · 2017-04-17T12:19:34Z

i think this is a good place to start writing a new traits ImageFile(types=['nifti-pair', 'nifti1'], compressed=False) and replace the File trait in the SPM tools for in files with this new trait.

also SPM can accept list of lists of 3D files so the check in this PR has to account for those scenarios. this is another reason to write a new Trait.

mgxd · 2017-04-18T17:50:04Z

@satra maybe something like this? only in this case, file endings are checked instead of types - any recommendation for that?

class ImageFile(File):
    """ Defines a trait of specific neuroimaging files """

    def __init__(self, value='', filter=None, auto_set=False, entries=0,
                 exists=False, types=[], compressed=True, **metadata):
        """ Trait handles neuroimaging files.

        Parameters
        ----------
        types : list
            The accepted file-types
        compressed : boolean
            Indicates whether the file-type can compressed
        """
        self.types = types
        self.compressed = compressed
        super(ImageFile, self).__init__(value, filter, auto_set, entries, exists, **metadata)

    def validate(self, object, name, value):
        """ Validates that a specified value is valid for this trait.
        """
        validated_value = super(ImageFile, self).validate(object, name, value)
        if validated_value and self.types:
            if self.compressed:
                self.types.extend([x + '.gz' for x in self.types])
            if not any(validated_value.endswith(x) for x in self.types):
                raise TraitError(
                    args="{} is not included in allowed types: {}".format(
                        validated_value, ','.join(self.types)))
        return validated_value

satra · 2017-04-19T20:07:32Z

nipype/interfaces/traits_extension.py

+        validated_value = super(ImageFile, self).validate(object, name, value)
+        if validated_value and self.types:
+            if self.compressed:
+                self.types.extend([x + '.gz' for x in self.types])


how would this handle mgz?

that case would have to added into types

@mgxd - are you planning to fix this?

yes, i'll revisit it today

satra · 2017-04-19T20:23:51Z

nipype/interfaces/traits_extension.py

+        """ Validates that a specified value is valid for this trait.
+        """
+        validated_value = super(ImageFile, self).validate(object, name, value)
+        if validated_value and self.types:


can you check if self.types always returns True even if the metadata is not there? in the past traits would some weird things like that.

yeah - if types is defined as an empty list, it'll return False

satra · 2017-04-19T20:24:48Z

nipype/interfaces/traits_extension.py

+    """ Defines a trait of specific neuroimaging files """
+
+    def __init__(self, value='', filter=None, auto_set=False, entries=0,
+                 exists=False, types=['nii', 'hdr', 'img'], compressed=True,


the reason i would suggest going with file format types instead of extensions is that programs that accept nifti-1 may not be able to run with nifti-2.

so here is an incomplete list: http://www.reproducibleimaging.org/module-dataprocessing/03-data/ (see mri data section)

and here is a description of nifti-1 vs nifti-2 check for future reference (i don't want to implement all validators right now).

link to nifti1 vs nifti2?

maybe we can work around this by adding a field to ImageFile that specifies format_version?

https://brainder.org/2015/04/03/the-nifti-2-file-format/

say one of the other interfaces (e.g., dcm2niix) were to restrict input files to dicoms. how would you determine that the input was valid for the MGH dicoms for example, which don't have extensions?

i'm also thinking of how we can use this interface to auto-(un)compress eventually instead of restricting it.

further compressed nifti pair would involve img/hdr.gz pairs. if we check only the header, then spm could still run into trouble.

so types can be format with details about compressed and uncompressed being something like this to start with:

{'nifti1': [('.nii', '.nii.gz'), (('.hdr', '.img'), ('.hdr', '.img.gz'))], 'mgh': [('.mgh', '.mgz')], ... }

at least for now, since this solution is focused on spm, let's use Nifti1 as the type and check for the various combinations listed above.

@gllmflndn: does spm now support nifti-2?

SPM12 should read NIfTI-2 files - I don't think it has been used much yet though.

oesteban · 2017-04-19T21:35:16Z

Since you are working on this new trait, I have a thought since long ago about the File trait. This could be a good PR for it.

Can we please change the messages when the trait validation fails?

if the problem is the type, keep the message.
BUT if it didn't validate because the metadata exists is True but the file is not found, could we just say that the file was not found?. The value XX is not str or unicode message is very confusing.

mgxd · 2017-04-25T20:31:36Z

nipype/interfaces/traits_extension.py

+# each element consists of :
+# - uncompressed (tuple[0]) extension
+# - compressed (tuple[1]) extension
+img_fmt_types = {


@satra I haven't included validation yet, but LMKWYT

satra · 2017-04-25T21:32:04Z

nipype/interfaces/traits_extension.py

+            if fmt in img_fmt_types:
+                exts.extend(sum([[u for u in y[0]] if isinstance(y[0], tuple)
+                            else [y[0]] for y in img_fmt_types[fmt]], []))
+                if self.compressed:


compressed=True should allow for compressed and normal. perhaps we should change compressed to allow_compressed

it does - it will extend on the normal

satra · 2017-04-25T21:32:36Z

nipype/interfaces/traits_extension.py

+        'nifti2': [('.nii', '.nii.gz'),
+                   (('.hdr', '.img'), ('.hdr', '.img.gz'))],
+        'cifti2': [('.nii', '.nii.gz'),
+                   (('.hdr', '.img'), ('.hdr', '.img.gz'))],


cifti doesn't allow hdr-img i think - can you check.

looks like not supported in nifti2 or cifti2. also should img/hdr be separated as it's own ANALYZE format?

i don't think we should support analyze :)

satra · 2017-04-30T00:58:40Z

@mgxd - looks good. the new traits need some tests.

djarecka · 2017-05-26T12:22:58Z

nipype/interfaces/traits_extension.py

+                                        exists, **metadata)
+
+    def grab_exts(self):
+        # TODO: file type validation


@mgxd : are you still planning to add something more to this validation? or should I remove TODO?

Right now we are only checking the extensions, but in the future it would be nice to check bit differences (like say between nifti1 and nifti2)

fix: catch compressed files for spm realign

32ea82d

effigies reviewed Apr 14, 2017

View reviewed changes

Merge branch 'fix/realign' of github.com:mgxd/nipype into fix/realign

a2c3f28

mgxd added 2 commits April 19, 2017 12:12

enh: new traits applied to spm

fdd7e8b

Merge branch 'master' of git://github.com/nipy/nipype into fix/realign

0de0026

mgxd changed the title ~~enh: catch compressed files for spm realign~~ enh: trait for imaging files + implementation in SPM preproc Apr 19, 2017

satra reviewed Apr 19, 2017

View reviewed changes

wip: check against image format

3ec370c

mgxd commented Apr 25, 2017

View reviewed changes

Merge branch 'master' of git://github.com/nipy/nipype into fix/realign

519e96c

satra reviewed Apr 25, 2017

View reviewed changes

mgxd added 3 commits April 26, 2017 13:55

enh: imagetype base for spm, new image formats, allow_compression

e6626af

fix: trailing parenthesis

a2f434f

fix: clarity allow_compression

7100768

mgxd added the needs-review label Apr 26, 2017

mgxd added 2 commits May 1, 2017 10:43

Merge branch 'master' of git://github.com/nipy/nipype into fix/realign

d879587

enh: preliminary testing + altered auto tests

bf564f7

satra merged commit 26e8c12 into nipy:master May 3, 2017

mgxd deleted the fix/realign branch May 18, 2017 21:19

mgxd removed the needs-review label May 18, 2017

djarecka reviewed May 26, 2017

View reviewed changes

djarecka mentioned this pull request Jul 19, 2017

different error message from SPM interaces #2120

Closed

effigies mentioned this pull request Jan 26, 2018

With NiPype Memory with version 0.12.1 and >=0.13.0 #2406

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enh: trait for imaging files + implementation in SPM preproc #1949

enh: trait for imaging files + implementation in SPM preproc #1949

mgxd commented Apr 11, 2017

codecov-io commented Apr 11, 2017 •

edited

Loading

effigies Apr 14, 2017

satra commented Apr 17, 2017

mgxd commented Apr 18, 2017 •

edited

Loading

satra Apr 19, 2017

mgxd Apr 19, 2017

satra Apr 24, 2017

mgxd Apr 24, 2017

satra Apr 19, 2017

mgxd Apr 19, 2017

satra Apr 19, 2017

mgxd Apr 19, 2017

satra Apr 19, 2017

satra Apr 19, 2017

satra Apr 19, 2017

gllmflndn Apr 20, 2017

oesteban commented Apr 19, 2017

mgxd Apr 25, 2017

satra Apr 25, 2017

mgxd Apr 25, 2017

satra Apr 25, 2017

mgxd Apr 26, 2017

satra Apr 26, 2017

satra commented Apr 30, 2017

djarecka May 26, 2017

mgxd May 26, 2017

enh: trait for imaging files + implementation in SPM preproc #1949

enh: trait for imaging files + implementation in SPM preproc #1949

Conversation

mgxd commented Apr 11, 2017

codecov-io commented Apr 11, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

satra commented Apr 17, 2017

mgxd commented Apr 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oesteban commented Apr 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

satra commented Apr 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Apr 11, 2017 •

edited

Loading

mgxd commented Apr 18, 2017 •

edited

Loading