-
Notifications
You must be signed in to change notification settings - Fork 533
[ENH] Add interfaces wrapping DIPY worflows #2830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2830 +/- ##
==========================================
+ Coverage 67.46% 67.55% +0.09%
==========================================
Files 341 342 +1
Lines 43347 43622 +275
Branches 5377 5452 +75
==========================================
+ Hits 29243 29470 +227
- Misses 13407 13454 +47
- Partials 697 698 +1
Continue to review full report at Codecov.
|
Hi Serge, I think we can shoot for that. I'm not sure how much energy everyone has right now, but we've got a little over a week before we should freeze new merges. |
Ok, Thank you for your feedback Chris! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM. A couple questions on your log messages.
@skoudoro Just a quick bump. I think this is basically ready to go if you want to look over my suggestions. |
Co-Authored-By: skoudoro <[email protected]>
Co-Authored-By: skoudoro <[email protected]>
Thank you for the review and sorry for the late answer, I was travelling. I applied your suggestion, it was a good catch. @arokem and I were wondering if it is possible to automate this task for each DIPY release, or during nipype installation. Indeed, It should be easy to catch all workflows from DIPY and then, apply Any suggestions are welcomed! |
Even at import time! Nipype would import dipy and traverse the workflows
namespace, finding names that conform with a particular pattern and making
the interfaces from these names.
I'd love to hear whether you think this is plausible, but I don't
it's necessary to do now. I think that can be a refactor of the work here.
…On Wed, Jan 23, 2019 at 8:43 AM Serge Koudoro ***@***.***> wrote:
Thank you for the review and sorry for the late answer, I was travelling.
I applied your suggestion, it was a good catch.
@arokem <https://github.com/arokem> and I were wondering if it is
possible to automate this task for each DIPY release, or during nipype
installation. Indeed, It should be easy to catch all workflows from DIPY
and then, apply dipy_to_nipype function.
Any suggestion is welcomed!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2830 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAHPNkooWwG9wtzShlJGueLot3cy6Z5Gks5vGJEjgaJpZM4ZUPEO>
.
|
@arokem I agree with that approach. It might also be useful to have a registry, though, that associates interfaces with minimum required dipy versions, so that we can give a useful error when somebody tries to use a more recent workflow with an older dipy installed. I also agree that that's a problem for another day. |
Oh, and yes, that seems completely plausible. |
1.1.8 (January 28, 2019) * FIX: ANTS LaplacianThickness cmdline opts fixed up (nipy#2846) * FIX: Resolve LinAlgError during SVD (nipy#2838) * ENH: Add interfaces wrapping DIPY worflows (nipy#2830) * ENH: Update BIDSDataGrabber for pybids 0.7 (nipy#2737) * ENH: Add FSL `eddy_quad` interface (nipy#2825) * ENH: Support tckgen -select in MRtrix3 v3+ (nipy#2823) * ENH: Support for BIDS event files (nipy#2845) * ENH: CompositeTransformUtil, new ANTs interface (nipy#2785) * RF: Move pytest and pytest-xdist from general requirement into tests_required (nipy#2850) * DOC: Add S3DataGrabber example (nipy#2849) * DOC: Skip conftest module in API generation (nipy#2852) * DOC: Hyperlink DOIs to preferred resolver (nipy#2833) * MAINT: Install numpy!=1.16.0 from conda in Docker (nipy#2862) * MAINT: Drop pytest-xdist requirement, minimum pytest version (nipy#2856) * MAINT: Disable numpy 1.16.0 for Py2.7 (nipy#2855) * tag '1.1.8': (79 commits) MNT: Add @feilong to .zenodo, update ordering MNT: Update .mailmap MNT: Update .zenodo ordering Accept invitation as Zenodo release co-author (see nipy#2864) MAINT: Update .mailmap BF: allowing bids_event_file as alternate input MNT: Update .zenodo ordering MNT: Version 1.1.8 DOC: 1.1.8 changelog Update nipype/interfaces/dipy/tracks.py Update nipype/interfaces/dipy/reconstruction.py MNT: Install numpy!=1.16.0 from conda in Docker Add FSL auto test remake specs Update nipype/interfaces/io.py Remove return type named tuple Update nipype/info.py STY: Whitespace, line length Remove out_ prefix from EddyQuad outputs Apply minor edits from code review ...
Hi Nipype Devs,
The main goal of this PR is to show how to convert a DIPY command line (named workflow) to a Nipype interface. This is a follow up from our discussion with @satra, @arokem, @Garyfallidis and me.
This PR adds :
TODO: