Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

PR 2 of 4 for SNV Consensus: Merge callers script #184

Merged
merged 180 commits into from
Oct 30, 2019

Conversation

cansavvy
Copy link
Collaborator

Purpose/implementation

This is a side analysis that originated from this original PR but after some reorganization has been split up and spread out.

This second PR merges the callers' VAF and TMB files for an output of four files. These four files can optionally be used in a later script to make plots and/or be used by a later script to make the final mutation consensus call.

The four output files:

  • all_callers_vaf.rds - contains all the VAF file information for all callers.
  • all_callers_tmb.rds - contains all the TMB file information for all callers.
  • mutation_id_list.rds - a full list of the mutations that can be used for an UpSetR graph
  • callers_per_mutation.rds - contains a breakdown for each mutation of what callers called it. Will be used to identify the consensus mutations.

Issue

For SNV caller comparison #161, #103 and Tumor Mutation Burden #3 and sort of #11

Directions for reviewers

I don't like having to use suppressWarnings to have it ignore combining columns. Is there a more seamless way to make this section less "shoot-in-the-foot" like?
Some of the writing steps are quite memory intensive and will undoubtedly add to the run time of the CircleCI tests. Do you have suggestions for decreasing the run time for this script?

Results

This script doesn't add results, but is a the next step toward getting the snv consensus.

Docker and continuous integration

These items were taken care of in #183

  • The dependencies required to run the code in this pull request have been added to the project Dockerfile.
  • This analysis has been added to continuous integration.

PR Checklist

  • Run a linter
  • Set the seed- I didn't, but let me know if I should.
  • Comments and/or documentation up to date
  • Double check your paths
  • Spell check any Rmd file or md file
  • Restart R and run all notebooks fresh and save

@cansavvy cansavvy changed the title PR 2 of 4: SNV Consensus: Merge callers script PR 2 of 4 for SNV Consensus: Merge callers script Oct 30, 2019
# -v results \
# -o strelka2 \
# -s wxs \
# -w
Copy link
Member

Choose a reason for hiding this comment

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

What does this -w correspond to?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I used to have --overwrite have a short flag but then I deleted it. Forgot to delete it from the example.

Copy link
Member

@jaclyn-taroni jaclyn-taroni 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 changes to documentation I would like to see before merging. This branch is difficult to test locally because it requires so much upstream stuff to have been run, so I have not pulled it locally and tested it out. It does seem reasonable though.

analyses/snv-callers/scripts/03-merge_callers.R Outdated Show resolved Hide resolved
analyses/snv-callers/scripts/03-merge_callers.R Outdated Show resolved Hide resolved
@jaclyn-taroni jaclyn-taroni merged commit 4e033ba into AlexsLemonade:master Oct 30, 2019
@cansavvy cansavvy deleted the merge_callers_script branch December 19, 2019 13:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants