Skip to content

Conversation

@servoz
Copy link
Collaborator

@servoz servoz commented Sep 4, 2023

Please consider this PR that will be fast to review:

I think there are two solutions.
1- Remove the out_prefix parameter and the prefix remains always at "w"
or
2- use the out_prefix parameter instead of the hard-coded "w" value.

I think the second solution is the right one.
I propose a slight modification to this effect.

Currently, for the Normalize12 process in spm, the user can define the out_prefix parameter, but this is not used to generate output, as it is hard-coded to 'w' in Normalize12._list_outputs().
@effigies
Copy link
Member

This seems reasonable, but does it work? I don't see where the updated file names would be passed to SPM to tell it to write to those files.

@servoz
Copy link
Collaborator Author

servoz commented Sep 11, 2023

Yes it is working.
Using this little change in nipype.interfaces.spm.Normalize12:

$ python Python 3.11.4 (main, Jun 7 2023, 00:00:00) [GCC 12.3.1 20230508 (Red Hat 12.3.1-1)] on linux Type "help", "copyright", "credits" or "license" for more information. >>> import nipype.interfaces.spm as spm >>> import os >>> matlab_cmd = '/data/softs/SPM12standalone/spm12_r7771_Linux_R2019b/spm12/run_spm12.sh /data/softs/MATLAB_Runtime/v97 script' >>> spm.SPMCommand.set_mlab_paths(matlab_cmd=matlab_cmd, use_mcr=True) >>> norm12 = spm.Normalize12() >>> norm12.inputs.jobtype = 'estwrite' >>> norm12.inputs.image_to_align = '/home/Desktop/testNipypeNormalize12/anat.nii' >>> norm12.inputs.out_prefix = 'test' >>> os.listdir('/home/Desktop/testNipypeNormalize12') ['anat.nii', 'ref'] >>> norm12.run() <nipype.interfaces.base.support.InterfaceResult object at 0x7f537d58fc10> >>> os.listdir('/home/Desktop/testNipypeNormalize12') ['y_anat.nii', 'anat.nii', 'ref', 'testanat.nii'] 

I don't see where the updated file names would be passed to SPM to tell it to write to those files.

I confess I didn't look into the nipype machinery to see exactly where the updated filenames would be passed to SPM to tell it to write to those files. ... However, concretely, with the small modification I propose, it works directly with nipype or in our environment (populse).

@effigies
Copy link
Member

Sounds good. Could you go ahead and run black on this file? My guess is we have a line length issue.

@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Patch coverage: 50.00% and no project coverage change.

Comparison is base (83d52f9) 63.18% compared to head (b9cac5e) 63.18%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@ ## master #3600 +/- ## ======================================= Coverage 63.18% 63.18% ======================================= Files 308 308 Lines 40809 40809 Branches 5652 5652 ======================================= Hits 25784 25784 Misses 14013 14013 Partials 1012 1012 
Files Changed Coverage Δ
nipype/interfaces/spm/preprocess.py 49.52% <50.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@servoz
Copy link
Collaborator Author

servoz commented Sep 11, 2023

Could you go ahead and run black on this file?

Done!

@effigies
Copy link
Member

Thanks!

@effigies effigies merged commit 661dd1d into nipy:master Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants