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

Add filter for non-synonymous mutations and re-run TMB with bug fix #728

Merged
merged 62 commits into from
Jul 15, 2020

Conversation

cansavvy
Copy link
Collaborator

@cansavvy cansavvy commented Jul 13, 2020

Purpose/implementation Section

What scientific question is your analysis addressing?

  • Adding an option to drop the synonymous mutations from the TMB calculations.
  • Re-running downstream TMB items after getting them updated.

I would like to do some more in depth comparison of the old and new results, but I will save that for a different PR so as to not have too many things at once.

What was your approach?

  • Added a filtering option --nonsynfilter which is a TRUE/FALSE on whether mutations should be filtered to be only non-synonymous mutations.
  • I re-ran both TCGA and PBTA TMBs
  • I re-ran the tmb-compare-notebook and adjusted some of the margins so it would be better aligned.
  • I re-ran the fig2-mutational-landscape.R figure again so it's updated.

What GitHub issue does your pull request address?

#726

Directions for reviewers. Tell potential reviewers what kind of feedback you are soliciting.

  • Is there a way I can make the margins adjust more automatically? It was quite a bit of finagling of margins and text size to get the plots to be aligned. Perhaps this is just how it has to be.
  • Anything I'm missing?

Is the analysis in a mature enough form that the resulting figure(s) and/or table(s) are ready for review?

Yes. The TMBs for the adults do seem slightly higher? That's good.

Results

What is your summary of the results?

Still figuring this out. It's not changed as I suspected it would be.
*Note that data files themselves are not pushed to GitHub but we will need to update them in the next data release.

Reproducibility Checklist

  • 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.

Documentation Checklist

  • This analysis module has a README and it is up to date.
  • This analysis is recorded in the table in analyses/README.md and the entry is up to date.
  • The analytical code is documented and contains comments.

yuankunzhu and others added 30 commits March 4, 2020 00:22
add folder for notebooks
add gitignore
update output file path handle in get-tcga-capture_kit.py
add boxplot.R
skip the manifest header
add ci for tcga-capture-kit-investigation
Merge branch 'analyses/tcga-capture-kit-investigation' of https://github.com/yuankunzhu/OpenPBTA-analysis into Teja_tmbQC
@cansavvy cansavvy changed the title Add filter for non-synonymous mutations and re-run TMB with bug fix WIP: Add filter for non-synonymous mutations and re-run TMB with bug fix Jul 13, 2020
@cansavvy cansavvy added the work in progress Used to label (non-draft) pull requests that are not yet ready for review label Jul 13, 2020
@cansavvy cansavvy marked this pull request as ready for review July 14, 2020 13:51
@cansavvy cansavvy changed the title WIP: Add filter for non-synonymous mutations and re-run TMB with bug fix Add filter for non-synonymous mutations and re-run TMB with bug fix Jul 14, 2020
@cansavvy cansavvy requested a review from jashapiro July 14, 2020 14:06
@cansavvy cansavvy removed the work in progress Used to label (non-draft) pull requests that are not yet ready for review label Jul 14, 2020
@cansavvy
Copy link
Collaborator Author

This is a reminder to myself to update the blog post with these updates to TMB. @cansavvy

Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

This looks great. Do we want to include a version without the nonsyn filter as well, just for comparison? I would not put that in the final figure, but having the figure and/or data table might be nice.

As to your question about margins, I might try (if you have not already) assembling the plot with patchwork or cowplot, both of which tend to handle adjusting parameters to make the grids line up pretty automatically.

Comment on lines +158 to +168
nonsynonymous <- c(
"Missense_Mutation",
"Frame_Shift_Del",
"In_Frame_Ins",
"Frame_Shift_Ins",
"Splice_Site",
"Nonsense_Mutation",
"In_Frame_Del",
"Nonstop_Mutation",
"Translation_Start_Site"
)
Copy link
Member

Choose a reason for hiding this comment

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

As we have discussed, this does differ slightly from the standard proposed in https://jitc.bmj.com/content/8/1/e000147#DC by the Friends of Cancer Research TMB Harmonization Project.

The relevant table is here: #725 (comment)

In particular, the FoCR group does not include Translation_Start_Site, Nonstop_Mutation or Splice_Site. I tend to think all of those are worth including, but I guess one simple question is how common these mutations actually are in the data set. Either way, we will want to standardize our criteria across analyses.

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 think this question is something I can ask in a next PR that does some more exploration. I'll file an issue that lays out what kinds of exploration analyses I think should be included.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Filed the issue: #729

@cansavvy
Copy link
Collaborator Author

This looks great. Do we want to include a version without the nonsyn filter as well, just for comparison? I would not put that in the final figure, but having the figure and/or data table might be nice.

I want to do this, but figured I should make it a separate PR so this one stays focused.

@cansavvy
Copy link
Collaborator Author

The follow up questions, I'll address on separate PRs, notoed on this issue here: #729

For wrapping up this current PR, I'll play around a bit with finding a better way to "automagically" align the tmb plots.

@cansavvy
Copy link
Collaborator Author

I tried patchwork. But am encountering an error like what's documented here: thomasp85/patchwork#148
I tried minimizing the theme arguments specified but it still didn't fix the problem.

@cansavvy
Copy link
Collaborator Author

@jashapiro I actually use cowplot in the tmb-compare-tcga notebook, but for whatever reason the align = "v" argument doesn't work. Let me know if you have insight into why its broken/being ignored by cowplot: https://github.com/AlexsLemonade/OpenPBTA-analysis/blob/master/analyses/tmb-compare-tcga/compare-tmb.Rmd#L141

@cansavvy
Copy link
Collaborator Author

Not sure what else to try for the "automagic" y axis alignment (see above comments). But in the interest of moving this and the future issues (#729) forward, I've wrote this problem down here: cansavvy/openpbta-notebook-concept#9 and will return to it if we have any breakthroughs.

With this being noted, are we ready to merge, @jashapiro ?

@jashapiro
Copy link
Member

With this being noted, are we ready to merge, @jashapiro ?
:shipit:

@jashapiro jashapiro merged commit 9b44bf1 into AlexsLemonade:master Jul 15, 2020
@cansavvy cansavvy deleted the cansavvy/non-syn-filter branch September 8, 2020 18:24
@jaclyn-taroni jaclyn-taroni mentioned this pull request Sep 11, 2020
9 tasks
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.

5 participants