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

Increase Code Coverage #80

Merged
merged 18 commits into from
Jan 27, 2019
Merged

Conversation

addisonElliott
Copy link
Collaborator

@addisonElliott addisonElliott commented Jan 26, 2019

Went through and added tests to uncovered routes according to codecov. Went from 88% to 90%!

Small other changes made along the way.

@codecov-io
Copy link

codecov-io commented Jan 26, 2019

Codecov Report

Merging #80 into master will increase coverage by 11.04%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #80       +/-   ##
===========================================
+ Coverage   88.12%   99.16%   +11.04%     
===========================================
  Files           6        6               
  Lines         362      361        -1     
  Branches      117      116        -1     
===========================================
+ Hits          319      358       +39     
+ Misses         21        1       -20     
+ Partials       22        2       -20
Impacted Files Coverage Δ
nrrd/parsers.py 100% <ø> (+1.72%) ⬆️
nrrd/writer.py 97.43% <ø> (+10.25%) ⬆️
nrrd/reader.py 100% <100%> (+16.26%) ⬆️

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 c1c42d9...32c6c6d. Read the comment docs.

elif field in []:
return 'int vector'
# elif field in []:
# return 'int vector'
elif field in ['space origin']:
return 'double vector'
elif field in ['measurement frame']:
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this the place where Isaiah and I corrected 'double matrix'? Not sure if you are working on a different branch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, this is the place. I just checked this PR and it's up to date. The measurement frame field type is 'double matrix' as corrected.


# Set the line skip to be incorrect
header['line skip'] = -1

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought I added this one before, but thanks if I haven't.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think you tested the byte skip and line skip for all encoding except raw. But that's one benefit of code coverage is for, telling us when something isn't tested.

@tashrifbillah
Copy link
Contributor

Don't see any obvious discrepancy. I would give it a green.

@addisonElliott
Copy link
Collaborator Author

@tashrifbillah Is this how long you meant by you needed time? That's nothing! I was expecting on the order of days to weeks!

Thank you for the review.

@addisonElliott addisonElliott merged commit e472ad8 into mhe:master Jan 27, 2019
@addisonElliott addisonElliott deleted the increase-codecov branch January 27, 2019 06:08
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.

3 participants