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

Do not warn about unknown states if ancestral allele is "N" #962

Closed
hyanwong opened this issue Sep 4, 2024 · 2 comments · Fixed by #963
Closed

Do not warn about unknown states if ancestral allele is "N" #962

hyanwong opened this issue Sep 4, 2024 · 2 comments · Fixed by #963

Comments

@hyanwong
Copy link
Member

hyanwong commented Sep 4, 2024

In #960 @benjeffery said:

This has made me realise the vcfzarr spec should probably be more explicit that zero-length alleles are not allowed. It is currently there as implicit as it states "" is a fill value.

In that case, I presume that we can always assume that an ancestral allele of "" means "intentionally marked as unknown", so that we can recommend that people use that to mark sites of unknown ancestral allele. Furthermore, I suspect that means we can exclude such alleles from the "warning" output that normally says:

An ancestral allele was not found in the variant_allele array for the {tot} sites ({frac_bad * 100 :.2f}%) listed below.

?

@hyanwong hyanwong changed the title Do not warn about unknown states if ancestral allele is "" Do not warn about unknown states if ancestral allele is "N" Sep 5, 2024
@hyanwong
Copy link
Member Author

hyanwong commented Sep 5, 2024

Revised in the light of discussions in #963 so that "N" should be the "deliberately unknown" state. We still want to treat "" as unknown, but this is more likely to be accidental.

@benjeffery
Copy link
Member

Yeah, I think you're right that we shouldn't warn on N

@mergify mergify bot closed this as completed in #963 Sep 10, 2024
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 a pull request may close this issue.

2 participants