Simple sv alt allele #1192
Simple sv alt allele #1192
Conversation
|
@droazen please review. |
|
@SHuang-Broad Yes, I just needed the constants. |
| @@ -200,6 +200,11 @@ protected Allele(final Allele allele, final boolean ignoreRefState) { | |||
| public final static Allele NO_CALL = new Allele(NO_CALL_STRING, false); | |||
| public final static Allele NON_REF_ALLELE = new Allele(NON_REF_STRING, false); | |||
|
|
|||
| public final static Allele SV_SIMPLE_DEL = new Allele(createBracketedSymbAlleleString(StructuralVariantType.DEL), false); | |||
There was a problem hiding this comment.
Can you just call toSymbolicAltAllele() on the enum values directly, and get rid of createBracketedSymbAlleleString()?
| public final static Allele SV_SIMPLE_INS = new Allele(createBracketedSymbAlleleString(StructuralVariantType.INS), false); | ||
| public final static Allele SV_SIMPLE_INV = new Allele(createBracketedSymbAlleleString(StructuralVariantType.INV), false); | ||
| public final static Allele SV_SIMPLE_CNV = new Allele(createBracketedSymbAlleleString(StructuralVariantType.CNV), false); | ||
| public final static Allele SV_SIMPLE_DUP = new Allele(createBracketedSymbAlleleString(StructuralVariantType.DUP), false); |
There was a problem hiding this comment.
Can you add a comment above each SV allele declaration, describing what kind of variant it represents?
Codecov Report
@@ Coverage Diff @@
## master #1192 +/- ##
===============================================
+ Coverage 68.406% 68.408% +0.002%
- Complexity 8017 8018 +1
===============================================
Files 542 542
Lines 32693 32701 +8
Branches 5530 5530
===============================================
+ Hits 22364 22370 +6
- Misses 8126 8127 +1
- Partials 2203 2204 +1
|
|
@droazen please review again. |
| @@ -200,6 +199,16 @@ protected Allele(final Allele allele, final boolean ignoreRefState) { | |||
| public final static Allele NO_CALL = new Allele(NO_CALL_STRING, false); | |||
| public final static Allele NON_REF_ALLELE = new Allele(NON_REF_STRING, false); | |||
|
|
|||
| // for simple deletion, e.g. "ALT==<DEL>" (not that the spec allows, for now at least, alt alleles like <DEL:ME>) | |||
There was a problem hiding this comment.
for myself: "not" -> "note"
| @@ -200,6 +199,16 @@ protected Allele(final Allele allele, final boolean ignoreRefState) { | |||
| public final static Allele NO_CALL = new Allele(NO_CALL_STRING, false); | |||
| public final static Allele NON_REF_ALLELE = new Allele(NON_REF_STRING, false); | |||
|
|
|||
| // for simple deletion, e.g. "ALT==<DEL>" (not that the spec allows, for now at least, alt alleles like <DEL:ME>) | |||
| public final static Allele SV_SIMPLE_DEL = new Allele(StructuralVariantType.DEL.toSymbolicAltAllele(), false); | |||
There was a problem hiding this comment.
Can't you just write public final static Allele SV_SIMPLE_DEL = StructuralVariantType.DEL.toSymbolicAltAllele(); here? (and similarly below)
There was a problem hiding this comment.
Opps. Went to quick at that. Yes you are right!
| * @throws UnsupportedOperationException if this is invoked on a {@link #BND} object | ||
| */ | ||
| Allele toSymbolicAltAllele() { | ||
| if (this.equals(StructuralVariantType.BND)) |
There was a problem hiding this comment.
Add curly braces around if statement body.
SHuang-Broad
force-pushed
the
sh-simple-sv-alt-allele
branch
from
c697223
to
7397272
Compare
|
@droazen I've squashed all commits into a single one. |
|
👍 looks good to me -- @lbergelson can you merge? |
Description
Simple alt allele constants for SV.
Two implementations, not sure which one is preferred.
Checklist