-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Major updates #34
Major updates #34
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really important set of improvements and serves as a great reminder for where documentation needs to still be filled in. Admittedly, mostly functions I've started and left for the time being.
#' @param bin_df Data frame with bins. | ||
#' @param min_value Minimum value to retain the bin. | ||
#' @param filename Path to a local file on drive to save the resulting file. | ||
#' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should say "nothing" for the return or (better yet) make functions like this one return the "full path to the file that was written"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, added this to this function and other similar functions that were writing files
R/cache_output.R
Outdated
#' @param function_params TODO. | ||
#' @param additional_details TODO. | ||
#' | ||
#' @return file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see earlier comment regarding what to return
@@ -31,7 +31,7 @@ | |||
#' maf1, | |||
#' regions_bed = grch37_ashm_regions | |||
#' ) | |||
#' #' calculate_tmb( | |||
#' calculate_tmb( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The title of this function should probably be "calculate tumour mutation burden", to be more intuitive and self-explanatory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - Updated the title!
This PR resolves the following issues: