Skip to content
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

Add support for HRF convolution #250

Closed
tyarkoni opened this issue Sep 13, 2018 · 4 comments
Closed

Add support for HRF convolution #250

tyarkoni opened this issue Sep 13, 2018 · 4 comments

Comments

@tyarkoni
Copy link
Collaborator

For a variety of reasons, it makes sense to move HRF convolution out of fitlins and into pybids. This will make it much easier to construct "final" design matrices we can just hand off to estimators like nistats. We can achieve this very easily by bundling @bthirion's hemodynamic_models module from nistats in pybids.

@bthirion, any objections? We could also add nistats as a dependency, but I don't think it makes sense to do that at the moment given that this is the only thing we need from nistats, and I'd like to avoid adding sklearn and nilearn as dependencies if we can help it.

@bthirion
Copy link
Contributor

I understand your concern: reducing the dependencies ! On the other hand, I'd surprised you don't use Nilearn in your data processing (if you don't you probably should ;-))

@tyarkoni
Copy link
Collaborator Author

pybids doesn't do any data processing; right now it never even reads an image (well, except to determine the number of volumes). I'd like to keep it that way if I can help it... :)

@effigies
Copy link
Collaborator

Would it make sense to just make the design matrix bits a small, self-contained package that pybids and nistats could each depend on? It should have an order of magnitude less churn than the rest of nistats, as it's (almost) exactly the same as what was in nipy, before.

@bthirion
Copy link
Contributor

I'd rather not: we realize that package maintenance is a burden, and we prefer to have fewer, well maintained packages rather than a bunch of small ones that nobody will take care of.
For this we reason, we'll actually merge Nistats into Nilearn in the next months (Yes, I've been saying that for months, but now we have somebody actively working on it) !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants