-
Notifications
You must be signed in to change notification settings - Fork 52
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
FIX: Check for valid qform before calculating change #466
Conversation
Best reviewed: commit by commit
Optimal code review plan
|
Hello @effigies, Thank you for updating! Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, Comment last updated at 2020-02-26 17:35:11 UTC |
53c7ea2
to
2878576
Compare
Hard one to test. I don't know how to create a file that doesn't have a valid qform. I've asked the submitter to send me their header, and maybe we can do something there. |
d2440f9
to
579c819
Compare
Now have full coverage of this interface. See https://codecov.io/gh/poldracklab/niworkflows/pull/466/src/niworkflows/interfaces/images.py?before=niworkflows/interfaces/images.py This is ready for a review. |
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.
Looks great!
20.0.1 (February 27, 2020) Bug-fix release in 20.0.x series. This release includes fixes for rare images with invalid qform matrices and some minor improvements in report readability and inclusion of common templates in the Docker image. * FIX: Handle qforms with invalid quaternions (nipreps/niworkflows#466) * FIX: update niworkflows location (#2005) * ENH: Display errors as summary/details elements in reports (nipreps/niworkflows#464) * DOC: Add ``--fs-subjects-dir`` usage to slurm example (#2003) * CI: Test that Docker image can run a common set of output spaces without network access (#1997)
This is a follow-up to #365. We were running into the case where
valid_qform
wasFalse
, and thusqform
was an unbound variable, but trying to compare the new qform to the old qform.This bug was introduced in 92e63fe, first released in 0.10.3. As this was included in fMRIPrep as of nipreps/fmriprep@b6d1e51, this bug is expected to affect versions 1.5.0-20.0.0 of fMRIPrep.
Fixes #465.