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

Added a small try-catch statement for corrupted or empty NIfTI files and test #51

Merged
merged 3 commits into from
Apr 2, 2021
Merged

Conversation

shak360
Copy link
Contributor

@shak360 shak360 commented Apr 1, 2021

Added a try-catch block in the isgz function to capture scenarios when NIfTIs are corrupted or empty.

Added a test for this and an empty.nii.gz file.

Deleted NIfTI files that weren't being used in testing (the exact same files were in the test/data folder).

svisagan@Shakthis-Mac-mini NIfTI % cmp -l test/data/example4d.hdr.gz test/example4d.hdr.gz # no output 
svisagan@Shakthis-Mac-mini NIfTI % cmp -l test/data/example4d.img.gz test/example4d.img.gz # no output
svisagan@Shakthis-Mac-mini NIfTI % cmp -l test/data/example4d.nii.gz test/example4d.nii.gz # no output

(The above implies those files were the same. They were also not being used in the code anywhere).

@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

Merging #51 (8111fea) into master (ad8e822) will decrease coverage by 0.30%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
- Coverage   43.27%   42.97%   -0.31%     
==========================================
  Files           4        4              
  Lines         372      370       -2     
==========================================
- Hits          161      159       -2     
  Misses        211      211              
Impacted Files Coverage Δ
src/NIfTI.jl 63.80% <100.00%> (-0.44%) ⬇️

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 ad8e822...8111fea. Read the comment docs.

ret
try
ret = read(io, UInt8) == 0x1F && read(io, UInt8) == 0x8B
seekstart(io)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also minute change, changed seek(io, 0) to seekstart(io)

Copy link
Member

Choose a reason for hiding this comment

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

Nice catch

@Tokazama
Copy link
Member

Tokazama commented Apr 2, 2021

Really appreciate this! Once this is merged I'm going to get one more quick commit in and release a patch bump for this.

@Tokazama Tokazama merged commit 9eaad22 into JuliaNeuroscience:master Apr 2, 2021
@shak360 shak360 deleted the shak-tiny-read-fix branch April 2, 2021 03:24
Tokazama added a commit that referenced this pull request Apr 2, 2021
Also changed nidim to to_dim_i16 b/c it is slightly faster and preps for
NIfTI-v2 headers
Tokazama added a commit that referenced this pull request Apr 2, 2021
Patch bump for #51 and changed `nidim` to `to_dim_i16` b/c it is slightly faster and preps for
NIfTI-v2 headers
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