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 function parse_variant to create a Struct with the locus and alleles from a variant string or contig, position, ref, and alt. #746

Merged
merged 4 commits into from
Dec 20, 2024

Conversation

jkgoodrich
Copy link
Contributor

No description provided.

…leles from a variant string or contig, position, ref, and alt.
Copy link
Contributor

@KoalaQin KoalaQin left a comment

Choose a reason for hiding this comment

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

Looks good, why did you add pytest for this one but not our other code?


@pytest.mark.parametrize(
"variant_str, contig, position, ref, alt, build, expected",
[
Copy link
Contributor

Choose a reason for hiding this comment

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

I've never used pytest myself, you only need to put some representative valid and invalid sennarios, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's just adding unit tests so that if we change the code and it breaks it, then hopefully the test will fail

@jkgoodrich
Copy link
Contributor Author

I just forgot to add tests to the other PR where I changed a function. We are trying to add tests now whenever we add a new function or change a function

Copy link
Contributor

@KoalaQin KoalaQin left a comment

Choose a reason for hiding this comment

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

LGTM!

@jkgoodrich jkgoodrich merged commit 727887c into main Dec 20, 2024
5 checks passed
@jkgoodrich jkgoodrich deleted the jg/add_variant_str_parser branch December 20, 2024 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants