Skip to content

FIX: properly handle V4 .PAR files with diffusion info #426

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

Merged
merged 2 commits into from
Mar 15, 2016

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Mar 14, 2016

This PR fixes a bug in get_bvals_bvecs for V4 .PAR files (the bug is only present for V4, not the more recent V4.1 or V4.2). In V4, the diffusion field does not exist and the bvecs cannot be determined.

no 'diffusion' field will be present in image_defs, so use None for the bvecs
The bvals can still be determined.

A V4 .PAR file test case was created by truncating the fields of the existing
V4.2 test case.
@grlee77 grlee77 changed the title FIX: properly handle of V4 .PAR files with diffusion info FIX: properly handle V4 .PAR files with diffusion info Mar 14, 2016
@grlee77 grlee77 mentioned this pull request Mar 14, 2016
@grlee77
Copy link
Contributor Author

grlee77 commented Mar 14, 2016

apologies... should have tested properly before uploading. I forgot that image_defs is a recarray and not a dictionary. will upload a fix later this evening

@matthew-brett
Copy link
Member

Thanks - looks good to me.

@grlee77
Copy link
Contributor Author

grlee77 commented Mar 15, 2016

I can rebase to a single commit if you prefer.

@matthew-brett
Copy link
Member

It's fine as is - was waiting to see if anyone else wanted to say something, but looks like no - merging.

matthew-brett added a commit that referenced this pull request Mar 15, 2016
MRG: properly handle V4 .PAR files with diffusion info

This PR fixes a bug in `get_bvals_bvecs` for V4 .PAR files (the bug is only present for V4, not the more recent V4.1 or V4.2).  In V4, the `diffusion` field does not exist and the `bvecs` cannot be determined.
@matthew-brett matthew-brett merged commit 87375ea into nipy:master Mar 15, 2016
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

Successfully merging this pull request may close these issues.

2 participants