Use Coordinates instead of Position #54
Conversation
Deprecate Position Replace CoordinateSystem, Position, Position with Coordinates in constructors
|
Hi @julesjacobsen , |
|
Hi @julesjacobsen , Should we coordinate this? I may do that except for pushing to Maven Central - I think you did that the last time |
|
Hi @ielis - Please hang on a bit as I need to check this out and test against a few of my projects first. I'm trying to finalize the never-ending Exomiser 13.0.0 release first, then I'll have a bit of time to spend on this. |
|
@julesjacobsen , this change is still hanging here in the void, and I think it would be good if we made our minds here. I favor going forward with removing How about you? What are your thoughs? |
| @@ -28,7 +27,32 @@ | |||
| "17, 198983, bndZ, C, .C, '', event5", | |||
| }) | |||
| public void roundTrip(String chr, int pos, String id, String ref, String alt, String mateId, String eventId) { | |||
| BreakendVariant variant = vcfConverter.convertBreakend(vcfConverter.parseContig(chr), id, Position.of(pos), ref, alt, ConfidenceInterval.precise(), mateId, eventId); | |||
| BreakendVariant variant = vcfConverter.convertBreakend(vcfConverter.parseContig(chr), id, pos, ConfidenceInterval.precise(), ref, alt, ConfidenceInterval.precise(), mateId, eventId); | |||
| //// TODO: Want to preserve input CHROM, POS, REF, ALT | |||
There was a problem hiding this comment.
@julesjacobsen Do we need to re-enable these tests? I do not think we actually need to test the variant attributes here, right? We test the correctness of the vcfConverter in VcfBreakendResolverTest. The point of this test seems to be the last assertion (line 56). If that's the case, we should remove the commented code. I did not want to do this without checking with you. :)
Thanks!
Hi @julesjacobsen ,
I did the monkey work and I replaced the
PositionbyCoordinates. All tests pass.Can we touch bases in the next days to look at the diffs/changes and either approve or discard the changes?
Cheers,
Daniel