-
Notifications
You must be signed in to change notification settings - Fork 2
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
VEP Refactoring #1 #123
base: master
Are you sure you want to change the base?
VEP Refactoring #1 #123
Conversation
.dropDuplicates(['varId']) | ||
# find variants in new datasets that aren't already in existing datasets | ||
df = broadcast(df).join(existing_variants_df, dataset_df.varId == existing_variants.varId, | ||
"leftanti") |
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.
Just as a curious note: is there a limit to the size that can be broadcast and if so what happens if you exceed it? Also did you test without broadcast and what happened / what was the difference?
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.
I did a tests at 1gb, 50gb, and 80gb, and they all worked and fairly fast too. I don't think I tried without broadcast.
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.
I can do more structured testing or we can just "do it live" with the release.
.csv(outdir, sep='\t') | ||
# if we find new variants, put them in the new_variants path for additional processing | ||
# and append them to the existing deduped variants path | ||
if not df.rdd.isEmpty(): |
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.
What would happen if this were removed? If an empty df is appended to something would it just have an extra empty file?
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.
Yeah, it will just add an empty file(s) to both places.
# just specify, potentially with a list only new datasets | ||
dataset_srcdir = f'{S3DIR}/variants/ExSeq/*/*' | ||
existing_variants = f'{S3DIR}/out/varianteffect/variants' | ||
new_variants = f'{S3DIR}/out/new-variants' |
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.
This should probably still be in out/varianteffect/new-variants
not in the first level of out, I imagine this is just an error carried from when you were writing to the test bucket
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.
Sounds good. I mean to call out for your opinion on the directories.
…tes the need for CQS and Common stages
No description provided.