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

FIX: Improve handling of optional fields in LTA #65

Merged
merged 3 commits into from
Mar 19, 2020

Conversation

effigies
Copy link
Member

@effigies effigies commented Mar 18, 2020

Closes #64.

No test right now, but it does handle the file provided there.

@pull-assistant
Copy link

pull-assistant bot commented Mar 18, 2020

Score: 1.00

Best reviewed: commit by commit


Optimal code review plan

     FIX: Allow optional fields to be missing in the middle of a file

     MNT: Require nibabel 3.0+

     TEST: Add simple regression test

Powered by Pull Assistant. Last update daa15a8 ... 2180e54. Read the comment docs.

@codecov-io
Copy link

Codecov Report

Merging #65 into master will increase coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #65      +/-   ##
==========================================
+ Coverage   98.52%   98.63%   +0.10%     
==========================================
  Files          11       11              
  Lines         951      950       -1     
  Branches      130      129       -1     
==========================================
  Hits          937      937              
  Misses          7        7              
+ Partials        7        6       -1     
Flag Coverage Δ
#unittests 98.63% <100.00%> (+0.10%) ⬆️
Impacted Files Coverage Δ
nitransforms/io/lta.py 99.32% <100.00%> (+0.66%) ⬆️

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 955cd38...8b58d4b. Read the comment docs.

Copy link
Collaborator

@oesteban oesteban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!

@oesteban oesteban merged commit 4acfdbc into nipy:master Mar 19, 2020
@effigies effigies deleted the fix/lta_missing_fields branch March 19, 2020 00:54
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.

Issue reading an LTA generated by fMRIPrep
3 participants