Skip to content

Conversation

@tclose
Copy link
Contributor

@tclose tclose commented Jul 26, 2016

The first argument to the latest specification of MRtrix 3's version of dwi2response is now a switch between 5 different methods for generating a diffusion response function. It appears that the ResponseSD interface in NiPype corresponds to the 'tax' option so I have added it into the _cmd attribute of the interface.

It may be desirable to include options for the different methods as well but I think these would work better as separate interfaces in any case.

@coveralls
Copy link

coveralls commented Jul 26, 2016

Coverage Status

Coverage remained the same at 72.325% when pulling 8dcc0f4 on tclose:mrtrix_dwi2response_update into 3ed411b on nipy:master.

@oesteban
Copy link
Contributor

oesteban commented Jul 26, 2016

Hi @tclose, would it be too hard to add a traits.Enum input (with default tax) to support the remaining function modes? Would it imply changes in other parameters?

(EDIT: I add the documentation file for convenience here https://github.com/MRtrix3/mrtrix3/blob/9a1dec74ef02231f21106dbed597911ba6fe886e/docs/reference/scripts/dwi2response.rst)

@tclose
Copy link
Contributor Author

tclose commented Jul 27, 2016

Hi @oesteban, it would not be too hard to add an enum for the differnt switches but then there are about 3-4 options per switch that are specific to that algorithm.

I reckon the best approach would be to create a base class with the common options and then derived classes for each of the switches since they are really different commands under the hood.

@effigies
Copy link
Member

This pull request is "orphaned," which means it has been deemed to be abandoned by its original author. Orphaned pull requests have not been rejected, and we hope that if a user sees one that will meet their needs with a little work, that they will fork it and open a new pull request (or, in the case of the original author, reopen the original PR).

We ask that all adopted PRs be updated to merge or rebase the current master. If you would like to adopt a PR and need help getting started, any of a number of contributors will be happy to help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants