search

nipy / nitransforms

a standalone fork of nipy/nibabel#656
https://nitransforms.readthedocs.io
MIT License
28 stars 17 forks source link

FIX: Misinterpretation of voxel ordering in LTAs #129

Closed oesteban closed 3 years ago

oesteban commented 3 years ago

The structarray was used directly and the extra axis actually changed the order of operations when the direction cosines were scaled by the voxel sizes.

A new test case has been added for which this error was apparent. This bug caused nipreps/fmriprep#2307, nipreps/fmriprep#2393, and nipreps/fmriprep#2410. nipreps/fmriprep#2444 just works around the problem by using lta_convert instead of NiTransforms. The lta_convert tool can be now dropped.

Resolves: #125

pep8speaks commented 3 years ago

Hello @oesteban! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2021-07-21 14:31:36 UTC
codecov-commenter commented 3 years ago

Codecov Report

Merging #129 (0ebf0e6) into master (0847e98) will decrease coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
- Coverage   98.89%   98.88%   -0.01%     
==========================================
  Files          12       12              
  Lines        1084     1081       -3     
  Branches      138      138              
==========================================
- Hits         1072     1069       -3     
  Misses          6        6              
  Partials        6        6
Flag Coverage Δ
travis 98.88% <100.00%> (-0.01%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
nitransforms/io/lta.py 99.32% <100.00%> (-0.02%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0847e98...0ebf0e6. Read the comment docs.

oesteban commented 3 years ago

Good to go? Do you feel the comments have been successfully addressed?

effigies commented 3 years ago

Sorry, just getting back to this this morning. Give me one hour.