-
Notifications
You must be signed in to change notification settings - Fork 16
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: Refactor of LTA implementation #145
Conversation
Codecov Report
@@ Coverage Diff @@
## master #145 +/- ##
==========================================
- Coverage 98.66% 98.28% -0.38%
==========================================
Files 12 12
Lines 1122 1109 -13
Branches 172 173 +1
==========================================
- Hits 1107 1090 -17
- Misses 5 11 +6
+ Partials 10 8 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
||
class LinearTransformArray(BaseLinearTransformList): | ||
class FSLinearTransformArray(BaseLinearTransformList): |
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.
Are we conflating FreeSurfer LTAs with something like a per-volume list of affines that one might use to motion-correct a BOLD series?
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.
I think so, if I understand correctly.
You said conflating because FreeSurfer LTA does not enforce that dst and src are the same for all the affines found in the file, right? while for motion-correct, those should be constant across affines - is that the question?
This has not been introduced by this PR, so I would prefer to address it on a separate issue.
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.
In the interest of making progress at other points and getting rid of the xfail marks asap, I'll go ahead and merge. |
This PR makes the LTA i/o implementation more consistent with the others, cleaning up some bugs, completing some missing aspects of the implementation (which help reduce the XFailed tests by 4).
Related: #40.