fix: MNIBiasCorrection smart out_file #1806

mgxd · 2017-02-10T16:50:09Z

codecov-io · 2017-02-10T23:03:24Z

Codecov Report

Merging #1806 into master will decrease coverage by -0.07%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #1806      +/-   ##
==========================================
- Coverage   73.09%   73.03%   -0.07%     
==========================================
  Files        1064     1064              
  Lines       53322    53329       +7     
==========================================
- Hits        38978    38947      -31     
- Misses      14344    14382      +38

Flag	Coverage Δ
#unittests	`73.03% <66.66%> (-0.07%)`	❌

Impacted Files	Coverage Δ
nipype/interfaces/freesurfer/preprocess.py	`68.15% <100%> (+0.19%)`	✅
...ype/interfaces/freesurfer/tests/test_preprocess.py	`91.83% <63.63%> (-8.17%)`	❌
nipype/utils/tests/test_filemanip.py	`80.54% <ø> (-12.67%)`	❌
nipype/init.py	`65.62% <ø> (-3.13%)`	❌
nipype/utils/tests/test_cmd.py	`95.65% <ø> (-2.9%)`	❌
nipype/pipeline/plugins/multiproc.py	`75.41% <ø> (-1.68%)`	❌
nipype/interfaces/utility/wrappers.py	`88.77% <ø> (-1.03%)`	❌
nipype/interfaces/tests/test_runtime_profiler.py	`43.57% <ø> (-0.56%)`	❌
nipype/utils/filemanip.py	`86.81% <ø> (-0.37%)`	❌

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 1a93a17...da1bd76. Read the comment docs.

…/mnibias

satra · 2017-02-11T13:46:12Z

nipype/interfaces/freesurfer/tests/test_preprocess.py

+
+    # test raising error with only partial mandatory args present
+    mni.inputs.in_file = filelist[0] #mgz
+    with pytest.raises(ValueError): mni.run()


this shouldn't raise an error. the point of having name_source and template is to autogenerate an out_file name. if that's not happening sth is broken.

also you shouldn't have to call run() for testing this. just, mni.cmdline

satra · 2017-02-14T22:29:22Z

nipype/interfaces/freesurfer/preprocess.py

@@ -1528,7 +1528,7 @@ class MNIBiasCorrection(FSCommand):
    output_spec = MNIBiasCorrectionOutputSpec

    def _list_outputs(self):
-        outputs = self._outputs().get()
+        outputs = self._output_spec().get()


this shouldn't need to be the case. what is failing with _outputs?

Since most other interfaces are using outputspec - I wanted to test. I can revert this

satra · 2017-02-14T22:29:49Z

nipype/interfaces/freesurfer/preprocess.py

@@ -1468,7 +1468,7 @@ class MNIBiasCorrectionInputSpec(FSTraitedSpec):
    in_file = File(exists=True, mandatory=True, argstr="--i %s",
                   desc="input volume. Input can be any format accepted by mri_convert.")
    # optional
-    out_file = File(argstr="--o %s", name_source=['in_file'], genfile=True,
+    out_file = File(argstr="--o %s", name_source=['in_file'],


why is genfile=True removed here?

I had added it in a previous test, but the out_file was still not being found

satra · 2017-02-16T02:51:46Z

nipype/interfaces/freesurfer/preprocess.py

@@ -1467,11 +1476,11 @@ class MNIBiasCorrectionInputSpec(FSTraitedSpec):
    # mandatory
    in_file = File(exists=True, mandatory=True, argstr="--i %s",
                   desc="input volume. Input can be any format accepted by mri_convert.")
-    out_file = File(argstr="--o %s", name_source=['in_file'],
+    # optional
+    out_file = File(argstr="--o %s", name_source=['in_file'], genfile=True,


genfile=True is not required. could you also merge with master?

satra · 2017-02-16T23:25:46Z

@oesteban - it seems like the docker container is being built every time. should that be happening?

satra · 2017-02-17T13:28:01Z

@oesteban - docker build error:

Step 26 : RUN conda config --add channels conda-forge --add channels intel &&     chmod +x /usr/local/miniconda/bin/* &&     conda config --set always_yes yes --set changeps1 no &&     conda update -q conda &&     chmod +x /usr/local/miniconda/bin/* &&     conda install -y mkl=2017.0.1                      numpy=1.11.2                      scipy=0.18.1                      scikit-learn=0.17.1                      matplotlib=1.5.3                      pandas=0.19.0                      libxml2=2.9.4                      libxslt=1.1.29                      traits=4.6.0                      psutil=5.0.1                      icu=58.1
 ---> Using cache
 ---> c388f2e830be
Step 27 : RUN sed -i 's/\(backend *: \).*$/\1Agg/g' /usr/local/miniconda/lib/python3.5/site-packages/matplotlib/mpl-data/matplotlibrc &&     python -c "from matplotlib import font_manager"
 ---> Running in 9f8f187d6533
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: No module named matplotlib

oesteban · 2017-02-17T18:25:57Z

For some reason I don't know, conda update is doing some funky stuff with the permissions. Matplotlib is installed but then permissions are too restrictive. @chrisfilo and I had to add this line for that very reason. That error happened before with the python 2.7 build. I'll fix that now

oesteban · 2017-02-17T18:57:30Z

Ok, now it is fixed: https://circleci.com/gh/oesteban/nipype/454 (to be merged within #1825)

mgxd · 2017-02-27T15:48:19Z

@satra this is all set

mgxd added 2 commits February 9, 2017 21:43

fix: decode instead of ignore

bf61ba6

fix: make outvol mandatory

18eedf7

mgxd changed the title ~~fix: make outvol mandatory~~ fix: make MNIBiasCorrection outvol mandatory Feb 10, 2017

enh: testing

2cecd53

mgxd added 4 commits February 10, 2017 18:03

Merge branch 'tst/travis' of github.com:mgxd/nipype into fix/mnibias

9990a53

Merge branch 'master' of https://github.com/nipy/nipype

1e19bca

Merge branch 'fix/mnibias' of https://github.com/mgxd/nipype into fix…

ebfbc42

…/mnibias

fix: doctest

4318274

satra reviewed Feb 11, 2017

View reviewed changes

mgxd added 10 commits February 14, 2017 12:35

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

aaecb34

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

5f040e9

enh: genfile

5782634

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

c62acee

fix: outputspec

fd1dda6

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

a56bfed

fix: outputspec attribute

cb74279

fix: grab out file

a5fc440

fix: mnibias test

6a44c6e

fix: removed changed file

b97a329

mgxd changed the title ~~fix: make MNIBiasCorrection outvol mandatory~~ fix: MNIBiasCorrection smart out_file Feb 14, 2017

satra reviewed Feb 14, 2017

View reviewed changes

mgxd added 2 commits February 15, 2017 10:44

fix: use _outputs()

8231470

fix: removed list outputs and test changes

8d935fb

satra reviewed Feb 16, 2017

View reviewed changes

mgxd added 2 commits February 16, 2017 14:20

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

2ef535b

removed genfile

ba1a863

oesteban added a commit to oesteban/nipype that referenced this pull request Feb 17, 2017

fix conda permissions (nipy#1806 / nipy#1806 (comment))

4503e9e

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

da1bd76

satra merged commit 1188b50 into nipy:master Feb 27, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: MNIBiasCorrection smart out_file #1806

fix: MNIBiasCorrection smart out_file #1806

Uh oh!

mgxd commented Feb 10, 2017 •

edited

Loading

Uh oh!

codecov-io commented Feb 10, 2017 •

edited

Loading

Uh oh!

satra Feb 11, 2017

Uh oh!

satra Feb 14, 2017

Uh oh!

mgxd Feb 15, 2017

Uh oh!

satra Feb 14, 2017

Uh oh!

mgxd Feb 15, 2017

Uh oh!

satra Feb 16, 2017

Uh oh!

satra commented Feb 16, 2017

Uh oh!

satra commented Feb 17, 2017

Uh oh!

oesteban commented Feb 17, 2017

Uh oh!

oesteban commented Feb 17, 2017

Uh oh!

mgxd commented Feb 27, 2017

Uh oh!

Uh oh!

fix: MNIBiasCorrection smart out_file #1806

fix: MNIBiasCorrection smart out_file #1806

Uh oh!

Conversation

mgxd commented Feb 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Feb 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

satra Feb 11, 2017

Choose a reason for hiding this comment

Uh oh!

satra Feb 14, 2017

Choose a reason for hiding this comment

Uh oh!

mgxd Feb 15, 2017

Choose a reason for hiding this comment

Uh oh!

satra Feb 14, 2017

Choose a reason for hiding this comment

Uh oh!

mgxd Feb 15, 2017

Choose a reason for hiding this comment

Uh oh!

satra Feb 16, 2017

Choose a reason for hiding this comment

Uh oh!

satra commented Feb 16, 2017

Uh oh!

satra commented Feb 17, 2017

Uh oh!

oesteban commented Feb 17, 2017

Uh oh!

oesteban commented Feb 17, 2017

Uh oh!

mgxd commented Feb 27, 2017

Uh oh!

Uh oh!

mgxd commented Feb 10, 2017 •

edited

Loading

codecov-io commented Feb 10, 2017 •

edited

Loading