ENH: Interfaces for MINC tools.

[carlohamalainen]

I wrote NiPype interfaces for some MINC tools when I ported volgenmodel to NiPype (https://github.com/carlohamalainen/volgenmodel-nipype).

Recently Chris Filo Gorgolewski asked me to submit a PR of my MINC interfaces as he is working on wrapping other MINC tools.

(This PR is a redo of #1302 in which I committed some large test files and then couldn't get github to drop the history with a rebase/squash.)

[chrisgorgo]

Could you rebase/ merge master? This would make the tests run correctly.

[carlohamalainen]

Did a rebase master, pull, push. Should be ok now.

[chrisgorgo]

Are you sure you pulled from nipy/nipype master? Did you push? The changes don't show on github.

[carlohamalainen]

All tests pass now.

[satra]

@carlohamalainen - one more quick change, could you please add an ENH line to the CHANGES file and point it to this PR URL.

[carlohamalainen]

@satra Done.

[satra]

@carlohamalainen - one more set of changes - this may take a bit of time.

we just went through an extensive PEP8 compliance PR for most of nipype. could we try to make as much of these files PEP8 compliant. the one PEP8 that i'm reasonably ok with violating is the hard 79 character limit. i'm ok with a fuzzy boundary on this as long as it improves readability.

in many cases you can get by with running automated tools such as autoflake8. but do be careful as some numerical processing things can get botched.

[carlohamalainen]

@satra autopep8 plus some manual edits did the trick. The only rule that is violated is the 79 character line limit.

[satra]

@carlohamalainen - thanks for the pep8 fixes.

github not showing the minc.py file is making this review a little more cumbersome. here are a few more things:

• docstrings and examples are missing for many of the interfaces.
• for the character limit, it would be good to at least break on the fields for the traits, so that one doesn't have to scroll a lot to see the rest.

for the desc field you can turn a string to a tuple like this:

nipype/nipype/interfaces/fsl/preprocess.py

Line 1500 in f9c98ba

desc=("Specify a numerical threshold value or use the 'method' input "

[carlohamalainen]

Why isn't github showing minc.py?

Anyway:

• All interfaces now have doctests.
• Long lines for traits are now split.
• Long desc fields now on multiple lines as suggested.

I also had a setup.py in the directory nipype/interfaces/minc which I think was a mistake. I removed that and added an entry for minc to the top-level setup.py.

[satra]

should be a relative import

[carlohamalainen]

@satra Imports are now relative.

[satra]

thanks @carlohamalainen

the reason that file is not showing is because the diff is greater than 100KB. github has some weird limitations.

here are a few more things to consider:

1. this section lists a number of FIXME's and TODO.

https://github.com/carlohamalainen/nipype/blob/carlo-minc-interface/nipype/interfaces/minc/minc.py#L44

1. this pattern of generating output filenames:

https://github.com/carlohamalainen/nipype/blob/carlo-minc-interface/nipype/interfaces/minc/minc.py#L263

can be simplified with name_source, name_template

http://www.mit.edu/~satra/nipype-nightly/devel/cmd_interface_devel.html#creating-outputs-on-the-fly

1. when you add extensions to files, do minc tools automatically generate the output file in the local directory? or do they generate the file in the same location as the source? say cwd = /some/cwd and input_file = /path/to/input_file is the output in /path/to/ or in /some/cwd? if the former (/path/to), then the metadata for input file should be augmented with copyfile=False. this symlinks the file to cwd when used in a workflow context.

the following link and notebook should help with explaining these metadata concepts.

http://www.mit.edu/~satra/nipype-nightly/devel/interface_specs.html

http://nbviewer.ipython.org/urls/dl.dropbox.com/s/mxud29oj2mxid6o/nipype_dev_tutorial.ipynb

[carlohamalainen]

1. The FIXMEs and TODOs were stale notes-to-myself; removed them.
2. Thanks for pointing out name_source and name_template! I was able to rip out a lot of redundant code.
3. I'm fairly sure that MINC tools place their output files in the CWD. Pinging @andrewjanke who can confirm?

[andrewjanke]

yes, CWD. If just a filename is given.

a
On 29/12/2015 8:37 PM, "Carlo Hamalainen" notifications@github.com wrote:

The FIXMEs and TODOs were stale notes-to-myself; removed them.
2.

Thanks for pointing out name_source and name_template! I was able to
rip out a lot of redundant code.
3.

I'm fairly sure that MINC tools place their output files in the CWD.
Pinging @andrewjanke https://github.com/andrewjanke who can confirm?

—
Reply to this email directly or view it on GitHub
#1304 (comment).

[satra]

thanks. the last bit that's needed is a tests directory with a blank __init__.py, with the directory then added to the main setup.py.

then in the main nipype directory do: make check

it will run everything and add a bunch of auto generated spec tests in the tests directory. git add/commit those and push

[carlohamalainen]

Do I need to commit the changes to nipype/algorithms/tests/test_auto_ErrorMap.py and nipype/algorithms/tests/test_auto_Overlap.py?

I've added tests directory etc.

Currently doing more tests with my volgenmodel workflow to make sure that everything's ok.

[carlohamalainen]

OK, volgenmodel workflow works, tests updated, should be good to go?