-
Notifications
You must be signed in to change notification settings - Fork 2
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
collate_igblast_results function #138
base: master
Are you sure you want to change the base?
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.
Some suggestions for how to improve efficiency and generalize
R/utilities.R
Outdated
@@ -1323,6 +1323,8 @@ collate_results = function(sample_table, | |||
these_samples_metadata, | |||
case_set, | |||
sbs_manipulation = "", | |||
mixcr_results_directory = "", |
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.
Parameters that are specific to functions being called by collate_results should really not be passed to them from this function. It's going to get too messy as the collate_ family grows. Remove from here but leave them as available parameters in the new function.
R/utilities.R
Outdated
#' Identify samples with mutated IGHV from MiXCR + IgBLAST results | ||
#' | ||
#' @param sample_table A data frame with sample_id as the first column. | ||
#' @param results_directory Directory containging MiXCR/IgBLASTN results. Depends on files having the columns "mutatedStatus" and "numMissingRegions" |
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.
Update docs after you make this optional so the users know they only need to specify it if they want to override the default
R/utilities.R
Outdated
@@ -1367,6 +1369,7 @@ collate_results = function(sample_table, | |||
sample_table = collate_ancestry(sample_table = sample_table, seq_type_filter = seq_type_filter) | |||
sample_table = collate_sbs_results(sample_table = sample_table, sbs_manipulation = sbs_manipulation, seq_type_filter = seq_type_filter) | |||
sample_table = collate_qc_results(sample_table = sample_table, seq_type_filter = seq_type_filter) | |||
sample_table = collate_igblast_results(sample_table = sample_table, seq_type_filter = seq_type_filter, results_directory = mixcr_results_directory, missing_threshold = missing_threshold) |
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.
Update code above to ensure the unix_group is always present in the samples table
R/utilities.R
Outdated
# Make table with necessary columns from result files | ||
mixcr_table = data.frame(biopsy_id=character(), | ||
mixcr_mutated=character(), | ||
missing=double()) |
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.
Start with a directory listing to avoid a loop. You probably will have to do this twice (once for each unix group). I think another recently added collate function does something similar
@mannycruz is this ready to go or is it still in development? |
@Kdreval Ready to go! |
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.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)