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

Remove non-Python dependencies #18

Closed
tsalo opened this issue Jun 20, 2022 · 7 comments · Fixed by #19
Closed

Remove non-Python dependencies #18

tsalo opened this issue Jun 20, 2022 · 7 comments · Fixed by #19
Milestone

Comments

@tsalo
Copy link
Contributor

tsalo commented Jun 20, 2022

I don't know if this is something you're interested in, but it would be awesome if pySPFM didn't require AFNI to run. I could be wrong, but it looks like the only places where AFNI is called are in hrf_afni and update_header:

https://github.com/eurunuela/pySPFM/blob/d69907cd3a8f0455b44504c74a73ee93915d4409/pySPFM/deconvolution/hrf_generator.py#L72

https://github.com/eurunuela/pySPFM/blob/dca806578adc86f55943a708d18bf02a2d4155ab/pySPFM/io.py#L62

It seems like hrf_afni could be replaced with something from nilearn.glm (e.g., nilearn.glm.first_level.spm_hrf). I don't know about update_header, but updating a HEAD/BRIK pair's header seems like something nibabel should be able to do.

@eurunuela
Copy link
Collaborator

I would love to make that 100% Pythonic. It would make the testing suite so much simpler.

I'll have a look at those. The good thing about using AFNI is it lets you choose the shape of the HRF. I don't know if the options provided by Nilearn are the same.

@tsalo
Copy link
Contributor Author

tsalo commented Jun 21, 2022

I think that Nilearn supports SPM and Glover HRFs, optionally with the temporal and/or dispersion derivatives. There's also an FIR option, but I don't think that's relevant here (I could be wrong). Are there other HRFs that are important for pySPFM? If so, I wonder if we could add them to Nilearn.

@eurunuela
Copy link
Collaborator

We have used the GAM HRF in AFNI's 3dDeconvolve.

Another option would be to let users input a .1D file with the HRF.

By the way, I don't think it's possible to add to the history on the header using nibabel.

@tsalo
Copy link
Contributor Author

tsalo commented Jun 21, 2022

The GAM HRF is the one that SPM uses, right? In that case, that should work. Nilearn can also handle custom HRFs as long as they're functions that predict values over time, if you wanted to define one of your own.

By the way, I don't think it's possible to add to the history on the header using nibabel.

Darn... maybe it's worth nibabel issue then.

@eurunuela
Copy link
Collaborator

eurunuela commented Jun 21, 2022

The GAM HRF is the one that SPM uses, right? In that case, that should work. Nilearn can also handle custom HRFs as long as they're functions that predict values over time, if you wanted to define one of your own.

Will have that in mind then. Thank you @tsalo, this is very useful!

Darn... maybe it's worth nibabel issue then.

Will open one!

Edit: opened nibabel #1119.

@eurunuela eurunuela added this to the v0.0.1 milestone Jun 22, 2022
@eurunuela eurunuela mentioned this issue Jun 22, 2022
4 tasks
@tsalo
Copy link
Contributor Author

tsalo commented Jun 28, 2022

What do you think of adding an option for generating BIDS-like sidecar files with the history, as an alternative? So folks could either use AFNI with subprocess or just pure Python, with an additional .json file in the output directory.

@eurunuela
Copy link
Collaborator

What do you think of adding an option for generating BIDS-like sidecar files with the history, as an alternative? So folks could either use AFNI with subprocess or just pure Python, with an additional .json file in the output directory.

That would be a good option.

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

Successfully merging a pull request may close this issue.

2 participants