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 public accessor for the per-target coverage information. #673

Merged
merged 2 commits into from Oct 18, 2016

Conversation

tfenne
Copy link
Collaborator

@tfenne tfenne commented Oct 14, 2016

I find that I'm using the HsMetricCollector and related classes programmatically a lot, and it would be really nice to be able to access the per-base and per-target coverage information that is accumulated without having to write it out to a file and parse it back in again. This PR adds a one-line accessor to make the information available to other classes.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 70.281% when pulling 6bb4cd1 on tfenne:tf_target_coverage_access into 4afc077 on broadinstitute:master.

@@ -406,6 +399,11 @@ public void setBaitSetName(final String name) {
this.metrics.PROBE_SET = name;
}

/** Returns the accumulated coverage per target. */
public Map<Interval,Coverage> getCoverageByTarget() {
Copy link
Contributor

Choose a reason for hiding this comment

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

-Space after comma,
-Note that the map is still mutable since it may change "from within" due to another call to acceptRecord. Could you add a comment to that effect?

otherwise, 👍

@tfenne
Copy link
Collaborator Author

tfenne commented Oct 18, 2016

Done @yfarjoun - will merge one tests go green again.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 70.281% when pulling 8cbf5fb on tfenne:tf_target_coverage_access into 4afc077 on broadinstitute:master.

@tfenne tfenne merged commit 4a55d81 into broadinstitute:master Oct 18, 2016
@tfenne tfenne deleted the tf_target_coverage_access branch October 18, 2016 21:12
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

3 participants