-
Notifications
You must be signed in to change notification settings - Fork 533
more concise and flexible parameter passing, increased parametrization #1749
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
@TheChymera - you should be able to run the test locally. we have updated the testing framework to use pytest. can you merge with master and try? simply |
963e0da
to
204cb1d
Compare
@satra , that's nice. I ran the tests locally, but again, the errors I get locally, with circleci, and travis, don't even remotely correspond to each other: At least with pytest, I think I can understand why it gives me these errors - because it checks dependencies, which on my system are different than what it expects to find in the container. Ideally, however, I would like to reproduce the circleci error locally. Perhaps you can help me out with it? It really sounds like the inputs are wrong :-/ but why then isn't this failing on master? |
@TheChymera - i found a few things to fix - give me a couple of days and i'll clarify the full testing on all platforms. some of the errors have to do with what dependencies are installed.
|
Thanks for helping me out with this. Could you show me how to run this particular test? Also, how exactly do I get the data, and where do I have t place it for nipype to find it? Regarding FSL, as you may recall from brainhack, I'm the package's maintainer on Gentoo. Please let me know if you figure out what I might have forgotten to make the package install. I guess something like that must be the issue. |
I understand that @satra is writing about the last py.test failure in
|
@djarecka - the |
@satra : ok, can change it |
@TheChymera - the instructions for the test data are in the circle.yml file. the initial part assigns URLs to environment variables (https://github.com/nipy/nipype/blob/master/circle.yml#L3). then, this section downloads and puts the data in specific locations (https://github.com/nipy/nipype/blob/master/circle.yml#L24). then running the tests is a matter of calling the relevant workflow pointing to each relevant location. so the fsl workflow needs the feeds data for example. regarding fsl version check, this is what nipype is doing: https://github.com/nipy/nipype/blob/master/nipype/interfaces/fsl/base.py#L70 |
@TheChymera - could you please merge with current master. re the circle error this is the relevant bit:
|
204cb1d
to
005e16e
Compare
Current coverage is 70.87% (diff: 100%)@@ master #1749 diff @@
==========================================
Files 1056 1028 -28
Lines 52745 50631 -2114
Methods 0 0
Messages 0 0
Branches 7663 7331 -332
==========================================
- Hits 38208 35883 -2325
- Misses 13326 13624 +298
+ Partials 1211 1124 -87
|
@satra I downloaded and extracted the data into the correct location, but the example script won't start. I am guessing this requires some magick to happen with my PYTHONPAHT? But I can't find which part of nipype does that. It's certainly not in the
|
if you look at the command that failed on circle there are a few more parameters:
|
I think it's looking for
|
@TheChymera - those are part of fsl. |
that file should be at: |
@satra I updated the Gentoo package accordingly. Now, however, it seems to be looking for the MNI atlas? Where should that be coming from?
|
that should be in: |
@satra ok, now that (ans all other files) should be in place. But now I get an entirely different error, which is again, not the one I am trying to reproduce:
I can't really see what is going wrong here, is it that this file Running the command from the command line gives me basically the same error:
|
@TheChymera - to ensure this is not an issue with your packaging, can you try it with a downloaded version of FSL binaries? |
005e16e
to
96d760b
Compare
@satra finally! |
Also, apparently this was an issue with |
The FSL model parameter passing to substitute the variables in the template EV files is very clunky and difficult to extend. I have streamlined parameter passing by putting all the EV file variable parameters in a dict. Also, using this new and easier paremeterization system, I have added parameters for gammadelay and gammasigma.
After this PR more even parameters can be added very easily and without cluttering the code.