morinlab / GAMBLR

Set of standardized functions to operate with genomic data
https://morinlab.github.io/GAMBLR/
MIT License
3 stars 2 forks source link

Adding error messages to functions that reads from local files. #117

Closed mattssca closed 1 year ago

mattssca commented 2 years ago

Pull Request Checklists

Important: When opening a pull request, keep only the applicable checklist and delete all other sections.

Checklist for all PRs

Required

This can be checked and addressed by running check_functions.pl and responding to the prompts. Test your code after you do this.

Optional but preferred with PRs

Checklist for New Functions

Required

Example:

#' Use GISTIC2.0 scores output to reproduce maftools::chromoplot with more flexibility
#'
#' @param scores output file scores.gistic from the run of GISTIC2.0
#' @param genes_to_label optional. Provide a data frame of genes to label (if mutated). The first 3 columns must contain chromosome, start, and end coordinates. Another required column must contain gene names and be named `gene`. (truncated for example)
#' @param cutoff optional. Used to determine which regions to color as aberrant. Must be float in the range [0-1]. (truncated for example)

Example:

#' @return nothing
#' @export
#' @import tidyverse ggrepel

Checklist for changes to existing code

PR Comments

This PR includes error messages for the following functions; get_ssm_by_samples get_ssm_by_sample get_gambl_metadata add_icgc_metadata (internal function for get_gambl_metadata) get_gambl_outcomes (internal function for get_gambl_metadata) get_combined_sv get_manta_sv get_sample_cn_segments get_coding_ssm annotate_ssm_blacklist collate_results assign_cn_to_ssm get_ssm_by_region get_excluded_samples

The above-mentioned functions have all been tested on remote GAMBLR and all return the added error message if Sys.setenv(R_CONFIG_ACTIVE = "remote" has not been specified.

Functions have also been tested after specifying the config to remote, and the collection of the above-mentioned functions runs the intended way, except for; get_combined_sv, get_sample_cn_segments, and get_ssm_by_region. I am currently looking into why the files needed for these functions are not accessible when running GAMBLR remote with a VPN connection.

Other small updates in this PR include typos in the README as well as removing trailing white spaces.

rdmorin commented 1 year ago

Is this ready for re-review?

mattssca commented 1 year ago

Yes, it's ready.

rdmorin commented 1 year ago

OK. Can you also please respond to my question above?

mattssca commented 1 year ago

This PR has now been updated based on the latest review comments, in this commit.