PBTA Histologies: Fusion filtering base (3 of N) #865
Conversation
With #793, we added |
There was a problem hiding this comment.
This looks good. My review is mainly two comments:
- I have the same comment about your if statements. If you just establish a histology file variable with your if statement, then you can have the main code written once. Since it looks like the histology file used is the only difference.
- I don't know enough about how we expected the base histology file to change these results, but as long as everything here is expected and makes sense then 👍.
| @@ -88,8 +96,13 @@ fusion_calls<-QCGeneFiltered_filtFusion %>% mutate(FusionName=rm_between(.data$F | |||
| group<-params$group | |||
|
|
|||
| # get histology file | |||
| if ( params$base_run ==0 ){ | |||
There was a problem hiding this comment.
Looks like the only difference here is whether params$base_histology or params$histology is used, so I would reduce your if statement to specify a histology_file variable and then supply that to that code chunk rather than repeating the code chunk twice.
| @@ -69,8 +77,14 @@ fusion_calls<-read_tsv(file.path(root_dir,params$dataPutativeFusion)) | |||
| outputfolder<-params$outputfolder | |||
|
|
|||
| #### get histology file | |||
| if ( params$base_run ==0 ){ | |||
| @@ -1,6 +1,5 @@ | |||
| FusionName broad_histology count | |||
| KIAA1549--BRAF Low-grade astrocytic tumor 109 | |||
| KIAA1549--BRAF Low-grade astrocytic tumor 114 | |||
There was a problem hiding this comment.
I'm assuming these results changes are expected, but just going to bring them to attention so we can check.
There was a problem hiding this comment.
yup, these are expected since some samples were updated to Low-grade. More info in Jo Lynne's comment here #865 (comment)
| @@ -26,7 +31,11 @@ set.seed(201910) | |||
|
|
|||
| ```{r load files} | |||
| root_dir <- rprojroot::find_root(rprojroot::has_dir(".git")) | |||
| if(params$base_run == 0) { | |||
There was a problem hiding this comment.
Same comment here about if statements and making it so you don't have to repeat the same code.
|
|
||
| if [[ RUN_FOR_SUBTYPING == "0" ]] |
There was a problem hiding this comment.
Same comment here about if statements. You can name a variable so you don't have to repeat whole chunks of code.
Purpose/implementation Section
What scientific question is your analysis addressing?
fusion_filteringresults/pbta-fusion-putative-oncogenic.tsv(included in data download)results/pbta-fusion-recurrent-fusion-byhistology.tsv(included in data download)results/pbta-fusion-recurrent-fusion-bysample.tsv(included in data download)results/fusion_summary_lgat_foi.tsv(included in data download)What was your approach?
Added condition to use pbta-histologies-base.tsv if running module for subtyping to 04-project-specific-filtering.Rmd,
05-QC_putative_onco_fusion_dustribution.Rmd and run_fusion_merged.sh.
What GitHub issue does your pull request address?
#861
Directions for reviewers. Tell potential reviewers what kind of feedback you are soliciting.
Which areas should receive a particularly close look?
Broad_histology column was updated in v18 changes in output files requiring broad_histology are seen in pbta-fusion-recurrent-fusion-byhistology.tsv. Some samples that were Neuronal and mixed neuronal-glial tumor are now grouped with Low-grade astrocytic tumor . CC @jharenza for additional info if required for review.
Other modules that need to be rerun for subtyping are in #861
Results
What types of results are included (e.g., table, figure)?
table
What is your summary of the results?
8 new stranded samples were added to the module and broad_histology were updated to match v18 base histology.
Reproducibility Checklist
Documentation Checklist
READMEand it is up to date.analyses/README.mdand the entry is up to date.