Error in MRTrix3 MRTransform

manuegrx commented 1 year ago

Hi,

I made a PR (https://github.com/nipy/nipype/pull/3611) in October but I forgot to make an issue and to add information to help to close the issue ! So here are the main elements :

Summary

Nipype MRTransform command does not work as desired. If several arguments are used (for example invert and linear_transform) they are not all take into account (and so it is not the same behavior as the native mrtrix3 command).

Actual behavior

If several arguments are used, only one is used by the Nipype command

Expected behavior

If several arguments are used, all should be used by Nipype command (and the result should be the same as the native mrtrix command)

How to replicate the issue


from nipype.interfaces import mrtrix3 as mrt

MRxform = mrt.MRTransform(invert=True)
MRxform.inputs.in_files = "path_to_file"
MRxform.inputs.linear_transform = "path_to_linear_transform_to_apply"
MRxform.inputs.out_file = "path_to_out_file"
MRxform.run()

Execution environment

My python environment outside container

How to fix the issue ?

I think that it is because all arguments are set to position = 1 I think it is not necessary to precise position for those arguments as argstr is already filled with an information about position

I changed that part in the PR

servoz commented 10 months ago

Dear nipype developers. We are using nipype in our project. We've an open ticket for this issue for several months (populse/mia_processes/#56). The cleanest solution would be to make the changes on the nipype side. Otherwise, we'll do the necessary overloading on our side. Could you please rule on the associated nipype PR so that we can close the ticket. Thank you in advance.

manuegrx commented 10 months ago

Hi,

Let me know if I can add some information or help to close the issue

Thank you,

servoz commented 8 months ago

Is the nipype project still alive?

effigies commented 8 months ago

Barely. Most development effort has gone towards pydra and I myself do not have much time for either. If you would like to do some maintenance work, I'd be happy to give you permissions, as someone who's been around for a while.

servoz commented 8 months ago

I understand the problem perfectly. Days are only 24 hours long and there are only 7 days in a week! I also don't have enough time to do everything I should be doing in my projects.

I thank you for your confidence, but at the moment I'm also finding it hard to cope with my workload. It would be detrimental to the quality of the result to add new tasks to my list at the moment.

Regarding this ticket and the associated PR, could you please merge the PR, or just let us know what's wrong? @manuegrx will make the necessary changes if required. In my opinion, the PR can be merged directly.

effigies commented 8 months ago

Done. I understand that your time is limited, so I'm not asking you to take on full maintenance. But if you agree, I can give you triage permissions so that you can mark PRs as approved.

servoz commented 8 months ago

Thanks for the merge!

I don't have much time, but I can certainly help. Nipype is a project that we use in our own and it's in our interest that it continues to develop.

effigies commented 8 months ago

@servoz I've given you write permissions, which includes triage and merge. Please do only what you want, but I do not want permissions to get in your way. And please feel free to ping me if you need feedback or sign-off on things. I will try to be responsive, but multiple pings may be needed at times.

nipy / nipype