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

Exclude MISSING when counting alleles per site #34

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

szhan
Copy link
Collaborator

@szhan szhan commented Apr 5, 2024

Fix #33

I reworked check_alleles in lshmm/api.py.

I implemented a helper function _get_num_alleles, which has the same logic as check_alleles, in the following two test files:

  • test_API.py
  • test_API_multiallelic.py

The above sets of tests pass.

I did not make any changes to test_LS_haploid_diploid.py, because there is no testing of FB when mutation rates are scaled by the number of distinct alleles.

Also, I'm only dealing with MISSING here. I'll add NONCOPY to the exclusion set in #31.

Copy link
Owner

@astheeggeggs astheeggeggs left a comment

Choose a reason for hiding this comment

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

This all looks good to me, defining a function to count the number of non-missing states in the reference, and count that as you track along the genome makes sense.

@szhan
Copy link
Collaborator Author

szhan commented Apr 16, 2024

Is this okay? If so, then perhaps we could merge it before I do some refactoring?

Copy link
Collaborator

@jeromekelleher jeromekelleher left a comment

Choose a reason for hiding this comment

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

LGTM

Shall we merge @astheeggeggs?

@astheeggeggs
Copy link
Owner

Sure, will merge

@astheeggeggs astheeggeggs merged commit ab8d4bb into astheeggeggs:main Apr 17, 2024
1 check failed
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.

MISSING as a distinct allele?
3 participants