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

Add BreakendParser and tests #37

Closed
wants to merge 1 commit into from

Conversation

ielis
Copy link
Collaborator

@ielis ielis commented Jan 15, 2021

Adding BreakendParser and extensive tests that exercise all possible breakend orientations as well as almost all lines of code.

@julesjacobsen
Copy link
Contributor

Thanks for the PR - I somehow didn't see this until now. Will merge in manually as there are conflicts. I think this is the final piece in the puzzle.

@ielis
Copy link
Collaborator Author

ielis commented Jan 19, 2021

I should have mentioned you.. 🙂 I'll do that from now on

throw new IllegalArgumentException("Unknown mate contig `" + mateContigName + '`');

Position matePos = null;
try {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looking at this now, we don't even need to be catching this. If the alt allele is matched by BND_ALT_PATTERN, then the content of the pos group should be able to be parsed into an integer. Unless it overflows the INT 😖

Copy link
Contributor

Choose a reason for hiding this comment

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

True, true - see 58359c2

@julesjacobsen
Copy link
Contributor

@ielis I've merged these changes renaming the new class as util/BreakendResolver like you originally wanted to name it.

I think we've addressed all the major issues, so can you try them out and then if we're looking good, lets do a 1.0.0 release!

@ielis ielis deleted the add_breakend_parser branch January 28, 2021 16:14
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.

2 participants