Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update to handle capture data #60

Merged
merged 21 commits into from
Mar 22, 2022
Merged

Update to handle capture data #60

merged 21 commits into from
Mar 22, 2022

Conversation

rdmorin
Copy link
Collaborator

@rdmorin rdmorin commented Mar 11, 2022

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.

  • I generated the documentation and checked for errors relating to the new function (e.g. devtools::document()) and added NAMESPACE and all other modified files in the root directory and under man.

Optional but preferred with PRs

  • I updated and/or successfully knitted a vignette that relies on the modified code (which ones?)

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:

#' 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)
  • My function uses a library that isn't already a dependency of GAMBLR and I made the package aware of this dependency using the function documentation import statment.

Example:

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

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)

Copy link
Contributor

@mattssca mattssca left a comment

Choose a reason for hiding this comment

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

Good updates, see comments for suggestions and questions. For me, I got an error when testing get_ssm_by_sample related to the maf path (saying the file does not exist). I updated my working directory but still got a similar error. For testing, I cloned this branch and ran the functions after loading this specific gamblr branch. This is how I ran the previously mentioned function ssm = get_ssm_by_sample(this_sample_id = "BLGSP-71-06-00001-01A-11D") while cd onto the cloned gamblr branch.

base_directory="/home/rmorin/",
my_name="Ryan Morin",
my_gitlab_email="rdmorin@sfu.ca"){
wflow_git_config(user.name = my_name, user.email = my_gitlab_email)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add overwrite option (default to FALSE?) to wflow_git_config()

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 I understand what causes this. Which "flavour" did you specify? Presumably just the default. Can you please retry with flavour="clustered"

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed!

After updating this line to if(this_pairing_status == "unmatched") and flavour = "clustered" the function runs the intended way.

#60 (comment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately I can't finalize and test the new version of this function without the final deblacklisted MAFs.


#' Set up a fresh instance of a website to host on gitlab
#'
#' @param site_base_name
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding descriptions of parameters, even though rather self-explenatory (to have consistent documentation generated).

#' @param site_base_name
#' @param base_directory
#'
#' @return
Copy link
Contributor

Choose a reason for hiding this comment

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

nothing

@rdmorin
Copy link
Collaborator Author

rdmorin commented Mar 22, 2022

I think this is good to go

@lkhilton lkhilton merged commit b1e9dc7 into master Mar 22, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants