-
Notifications
You must be signed in to change notification settings - Fork 31
Small variant annotation fix #223
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
Conversation
ch-kr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a couple small changes, but looks good to me
gnomad/variant_qc/pipeline.py
Outdated
| :param relationship_col: Column containing the relationship for the sample pair as defined in this module constants. | ||
| :return: A Table with the sibling shared variant counts | ||
| """ | ||
| mt = filter_to_autosomes(mt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add to the docstring that this currently only runs on autosomes?
|
Could you also add a note on the changes to the "Unreleased" section of CHANGELOG.md? That will help users see what changed between releases. |
nawatts
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
18ffb29 adds to the release notes for version 0.3, which has already been released. These should go in the unreleased section (currently empty) at the top.
ch-kr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a small comment, also agree with Nick that this should be under ##Unreleased
ch-kr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, but I do feel like sib stats and trio stats should be consistent
ch-kr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one small change to the changelog, then looks good to me
ch-kr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me!
| if autosomes_only: | ||
| mt = filter_to_autosomes(mt) | ||
| if bi_allelic_only: | ||
| mt = mt.filter_rows(bi_allelic_expr(mt)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for adding this!
No description provided.