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

Modified subset_samples_and_variants() #421

Merged
merged 14 commits into from
Dec 6, 2021

Conversation

wlu04
Copy link
Member

@wlu04 wlu04 commented Nov 17, 2021

Added a vds option to function subset_samples_and_variants() @jkgoodrich @klaricch

Copy link
Contributor

@jkgoodrich jkgoodrich left a comment

Choose a reason for hiding this comment

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

Thanks so much for starting to worth through these!!

gnomad/utils/filtering.py Outdated Show resolved Hide resolved
gnomad/utils/filtering.py Outdated Show resolved Hide resolved
gnomad/utils/filtering.py Outdated Show resolved Hide resolved
gnomad/utils/filtering.py Outdated Show resolved Hide resolved
gnomad/utils/filtering.py Outdated Show resolved Hide resolved
gnomad/utils/filtering.py Outdated Show resolved Hide resolved
gnomad/utils/filtering.py Outdated Show resolved Hide resolved
gnomad/utils/filtering.py Outdated Show resolved Hide resolved
wlu04 and others added 8 commits November 17, 2021 12:36
Co-authored-by: jkgoodrich <33063077+jkgoodrich@users.noreply.github.com>
Co-authored-by: jkgoodrich <33063077+jkgoodrich@users.noreply.github.com>
Co-authored-by: jkgoodrich <33063077+jkgoodrich@users.noreply.github.com>
Co-authored-by: jkgoodrich <33063077+jkgoodrich@users.noreply.github.com>
Co-authored-by: jkgoodrich <33063077+jkgoodrich@users.noreply.github.com>
Co-authored-by: jkgoodrich <33063077+jkgoodrich@users.noreply.github.com>
Copy link
Contributor

@jkgoodrich jkgoodrich left a comment

Choose a reason for hiding this comment

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

I think this PR just needs the addition of a remove_dead_alleles that can only be used if the input is a VDS, default set to False. I think you also need to pull and merge with the master branch so that the tests no longer fail, but after that it will be good to merge

Copy link
Contributor

@jkgoodrich jkgoodrich left a comment

Choose a reason for hiding this comment

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

A couple suggestions for the new addition

gnomad/utils/filtering.py Outdated Show resolved Hide resolved
gnomad/utils/filtering.py Show resolved Hide resolved
wlu04 and others added 2 commits December 6, 2021 15:09
Co-authored-by: jkgoodrich <33063077+jkgoodrich@users.noreply.github.com>
Co-authored-by: jkgoodrich <33063077+jkgoodrich@users.noreply.github.com>
Copy link
Contributor

@jkgoodrich jkgoodrich 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
Copy link
Contributor

Sorry, just need one more run of Black, then good to merge

@jkgoodrich jkgoodrich merged commit 978b577 into broadinstitute:master Dec 6, 2021
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.

3 participants