Create brainsuite.py #1305
Create brainsuite.py #1305
Conversation
I created an interface for Brainsuite's command line tools. I'd like it to be included in nipype builds installed per the instructions on the nipype website.
|
it would be good to run a PEP8 checker on this code. also in many cases you can use the simpler form for instantiating names of outputs using http://nipy.org/nipype/devel/cmd_interface_devel.html#creating-outputs-on-the-fly |
When I tried running this code, type(outputs[key]) was never == list or == str, so the method, self.s3tolocal, never got called. When I tried debugging, type(outputs[key]) turned out to be <type 'unicode'>
I updated the formatting to comply with the PEP8 style guide (The exception is that some lines run over 79 characters) As for instantiating names, the reason I approached it this way is to shorten the names of outputs that ran through multiple steps of processing using brainsuite command line tools. Is it fine to leave the instantiation of outputs how it is now?
|
I updated the formatting to comply with the PEP8 style guide (The exception is that some lines run over 79 characters) As for instantiating names, the reason I approached it this way is to shorten the names of outputs that ran through multiple steps of processing using brainsuite command line tools. Is it fine to leave the instantiation of output names how it is now? |
| else: | ||
| outputs['outputMRIVolume'] = self._gen_filename('outputMRIVolume') | ||
|
|
||
| if isdefined(self.inputs.outputMaskFile): |
There was a problem hiding this comment.
these almost seem like a pattern and i would like to simplify their creation. we can work on this after next week.
Added l_outputs function to simplify insantiation of output names/reduce repetition in code
|
@satra |
| ) | ||
|
|
||
|
|
||
| class bseInputSpec(CommandLineInputSpec): |
There was a problem hiding this comment.
Classes in the rest of Nipype codebase begin with a capitol letter...
|
Thanks, I removed the use of exec, and further simplified _gen_filename |
|
|
||
| inputMRIFile = File(exists=True, mandatory=True, argstr='-i %s', | ||
| desc='input MRI volume', position=0) | ||
| outputMRIVolume = File(mandatory=False, |
There was a problem hiding this comment.
the mandatory=False can all be removed.
|
Removed unnecessary metadata |
|
the next few steps are:
it would also be useful for future users to add an example for brainsuite, and add it to the examples folder, and test it using run_tests. make sure to modify .circle.yml to add installing brainsuite. before running before you do the above, you should decide if you want to isolate brainsuite in its own folder. this may be a reasonable thing if you are planning to dissociate components of brainsuite (e.g., see the spm, fsl, freesurfer, ants, interfaces) |
Added description and doctests
I created an interface for Brainsuite's command line tools. I'd like it to be included in nipype builds installed per the instructions on the nipype website.
When I tried running this code, type(outputs[key]) was never == list or == str, so the method, self.s3tolocal, never got called. When I tried debugging, type(outputs[key]) turned out to be <type 'unicode'>
I updated the formatting to comply with the PEP8 style guide (The exception is that some lines run over 79 characters) As for instantiating names, the reason I approached it this way is to shorten the names of outputs that ran through multiple steps of processing using brainsuite command line tools. Is it fine to leave the instantiation of outputs how it is now?
Added l_outputs function to simplify insantiation of output names/reduce repetition in code
Added description and doctests
|
@satra I ran make specs and make check-before-commit. Running make specs and then make check-before-commit also changed/created some other files that seemed to be unrelated to my interface, and I added them in 515c16e note: make check-before-commit resulted in some failures in a few doctests contained in files that I hadn't edited, for example: could not locate the file called ./xxx that is referenced in https://github.com/jason-wg/nipype/blob/patch-1/nipype/workflows/dmri/fsl/tbss.py Finally, yes, it's fine to keep Brainsuite in its own folder. |
|
@jason-wg - thanks - i see some changes for s3 in io.py. were these things you made? can you perhaps do a diff with an separate download of the master nipype to make sure. every once in a while github does some weird compares. |
|
@satra |
|
unfortunately github won't allow that through an automated merge. could you simply copy over the io.py and commit it and regenerate the tests or revert the datasink tests that were created. |
|
@satra |
|
thanks - a few more things.
|
|
@satra |
|
thanks @jason-wg - will wait for all the test to be done. |
|
@satra |
I created an interface for Brainsuite's command line tools. I'd like it to be included in nipype builds installed per the instructions on the nipype website.