Converted VariantCallingMetrics to using mergableMetricsBase #552
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| */ | ||
| public MergeableMetricBase merge(final Collection<? extends MergeableMetricBase> many) { | ||
| many.parallelStream().forEach(this::merge); | ||
| calculateDerivedFields(); |
There was a problem hiding this comment.
but the merge below does not call calculateDerivedFields! please remove.
|
@yfarjoun rebase onto master and I can revisit. The only comment that requires discussion is the one inside the merging of a collection ( |
yfarjoun
force-pushed
the
yf_MakeVariantCallingMetircsUseMergableMetrics
branch
from
f04be0b
to
183e6e7
Compare
|
@nh13 do you have a few minutes to review this old PR? |
nh13
approved these changes
Jan 9, 2017
| private static List<Field> getAllFields(Class clazz){ | ||
| List<Field> fields = new ArrayList<>(); | ||
| fields.addAll(Arrays.asList(clazz.getDeclaredFields())); | ||
| Class superClass = clazz.getSuperclass(); |
| long numHets, numHomVar; | ||
|
|
||
| public static String getFileExtension() { | ||
| return "variant_calling_detail_metrics"; | ||
| } | ||
|
|
||
| public static void foldInto(final VariantCallingDetailMetrics target, |
There was a problem hiding this comment.
Should be ok to delete instead of deprecate right?
- added tests - extended MergableMetricsBase so that it can deal with private and protected members of the superclass - cleaned up alignment/typos in FilterVcf and MergeBamAlignment (unrelated)
yfarjoun
force-pushed
the
yf_MakeVariantCallingMetircsUseMergableMetrics
branch
from
d9faa39
to
92ba5ec
Compare
|
This fell through the cracks. Did the changes requested, will merge after tests pass. |
|
This fell through the cracks. Did the changes requested, will merge after tests pass. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@nh13 you has said that you'd like to see this framework used for something other than IndependentReplicates...so, here's your chance! 😄