Closed mattssca closed 2 years ago
More to come this week, I'll ask for reviewers once the PR is ready to be merged. Thanks,
Also added updates to prettyOncoplot and annotate_hotspots provided by Kostia. package documentation has been regenerated.
FYI, After merging this branch with rmorin-dev, the following warnings were returned when generating package documentation:
Warning: [/home/cmattsson/GAMBLR/cmattsson-dev/GAMBLR/R/checks.R:15] @examples requires a value
Warning: [/home/cmattsson/GAMBLR/cmattsson-dev/GAMBLR/R/utilities.R:13] @examples requires a value
Warning: [/home/cmattsson/GAMBLR/cmattsson-dev/GAMBLR/R/utilities.R:1067] @param requires name and description
Warning: [/home/cmattsson/GAMBLR/cmattsson-dev/GAMBLR/R/utilities.R:1073] @examples requires a value
As of yesterday, I merged the active remote GAMBLR branch (5 commits behind that got pushed today) with this branch and resolved merge conflicts.
I have now done some rather extensive testing to ensure this update maintains existing functionality. This testing includes the following functions (directly affected by updates in this PR, i.e adding MAF column subset options). These functions were tested with a selection of different parameters and to my knowledge, the returns are matching the expected outputs.
get_ssm_by_samples
get_ssm_by_sample
get_ssm_by_patients
get_ssm_by_regions
get_ssm_by_region
get_coding_ssm
assign_cn_to_ssm
In addition, any functions calling any of the above-modified functions have also been tested, all returning expected outputs. These functions include:
get_gene_expression
calc_mutation_frequency_sliding_windows
get_ashm_count_matrix
copy_number_vaf_plot
ashm_multi_rainbow_plot
ashm_rainbow_plot
fancy_v_chrcount
cnv_vaf_plot
fancy_snv_chrdistplot
fancy_v_count
fancy_v_sizedis
fancy_ideogram
prettyRainfallPlot
Bumping a previous comment by me in this PR, If someone (@Kdreval or @rdmorin)could have a look at the recently added collate function as well as how the "master" collate function calls the recently added collate function (collate_qc_results) that would be greatly appreciated.
If we decide to add maf_columns as a global parameter (see my previous question in this PR), this would probably be best to address in an upcoming PR in order to get the changes made in this PR on to master asap and avoid future merge conflicts.
From my point of view, this PR could be merged into master if no outstanding issues remain. I'd be happy to take any questions or clarify if needed.
Thanks,
Pull Request Checklists
Important: When opening a pull request, keep only the applicable checklist and delete all other sections.
Checklist for all PRs
Required
[ ] I tested the new code for my use case (please provide a reproducible example of how you tested the new functionality)
[ ] I ensured all dplyr functions that commonly conflict with other packages are fully qualified.
This can be checked and addressed by running
check_functions.pl
and responding to the prompts. Test your code after you do this.devtools::document()
) and addedNAMESPACE
and all other modified files in the root directory and underman
.Optional but preferred with PRs
Checklist for New Functions
Required
[ ] I documented my function using ROxygen style.)
[ ] All parameters for the function are described in the documentation and the function has a decriptive title.
Example:
import
statment.Example:
Checklist for changes to existing code
[ ] I added/removed arguments to a function and updated documentation for all changed/new arguments
[ ] I tested the new code for compatability with existing functionality in the Master branch (please provide a reprex of how you tested the original functionality)