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

Adding mode and 95th percentile to InsertSizeMetrics. #1001

Merged
merged 3 commits into from Dec 15, 2017

Conversation

nh13
Copy link
Collaborator

@nh13 nh13 commented Nov 27, 2017

I find that I parse the histogram often for these metrics, and so I figured I should just go ahead and add them officially.

@coveralls
Copy link

coveralls commented Nov 27, 2017

Coverage Status

Coverage increased (+0.002%) to 77.223% when pulling 88e7b82 on nh_insert_size_mode_and_95_percentile into b0b9f78 on master.

@nh13
Copy link
Collaborator Author

nh13 commented Dec 15, 2017

@yfarjoun or @lbergelson bump?

@@ -168,6 +169,7 @@ public void addMetricsToFile(final MetricsFile<InsertSizeMetrics,Integer> file)
if (percentCovered >= 0.7 && metrics.WIDTH_OF_70_PERCENT == 0) metrics.WIDTH_OF_70_PERCENT = distance;
if (percentCovered >= 0.8 && metrics.WIDTH_OF_80_PERCENT == 0) metrics.WIDTH_OF_80_PERCENT = distance;
if (percentCovered >= 0.9 && metrics.WIDTH_OF_90_PERCENT == 0) metrics.WIDTH_OF_90_PERCENT = distance;
if (percentCovered >= 0.9 && metrics.WIDTH_OF_95_PERCENT == 0) metrics.WIDTH_OF_95_PERCENT = distance;
Copy link
Contributor

Choose a reason for hiding this comment

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

@nh13 shouldn't this be percentCovered >= 0.95 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No description provided.

@nh13
Copy link
Collaborator Author

nh13 commented Dec 15, 2017

@ktibbett one more time, omg that mistake.

@yfarjoun
Copy link
Contributor

I'm concerned that the tests passed with that error....could we have tests that would fail on that error?

@coveralls
Copy link

coveralls commented Dec 15, 2017

Coverage Status

Coverage increased (+0.4%) to 77.636% when pulling 1dc0c0f on nh_insert_size_mode_and_95_percentile into b0b9f78 on master.

@nh13 nh13 force-pushed the nh_insert_size_mode_and_95_percentile branch from acf57a1 to cf190f5 Compare December 15, 2017 19:26
@nh13
Copy link
Collaborator Author

nh13 commented Dec 15, 2017

@yfarjoun of course, done.

Assert.assertEquals(metric.WIDTH_OF_80_PERCENT, 1 + insertBy*14);
Assert.assertEquals(metric.WIDTH_OF_90_PERCENT, 1 + insertBy*16);
Assert.assertEquals(metric.WIDTH_OF_95_PERCENT, 1 + insertBy*18);
Assert.assertEquals(metric.WIDTH_OF_99_PERCENT, 1 + insertBy*20);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Look Ma, different values for each!

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@coveralls
Copy link

coveralls commented Dec 15, 2017

Coverage Status

Coverage increased (+0.002%) to 77.636% when pulling cf190f5 on nh_insert_size_mode_and_95_percentile into 1d95eed on master.

@coveralls
Copy link

coveralls commented Dec 15, 2017

Coverage Status

Coverage increased (+0.002%) to 77.636% when pulling cf190f5 on nh_insert_size_mode_and_95_percentile into 1d95eed on master.

@ktibbett
Copy link
Contributor

Thanks @nh13 : 👍

@nh13 nh13 merged commit cf26d12 into master Dec 15, 2017
@nh13 nh13 deleted the nh_insert_size_mode_and_95_percentile branch December 15, 2017 23:23
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.

None yet

4 participants