Conversation
… removal of log transformation
ALuesink
left a comment
ALuesink
left a comment
There was a problem hiding this comment.
Really a lot of work! It looks good, but I have a few specific comments and a few general comments.
General comments:
- Check the comments, some start with capital letters and others with lowercase letters.
- CollectSumAdducts.nf misses the change to de new Docker version.
- Check the linting of all the files.
| df_intensities <- as.data.frame(peakgroup_list[, col_idx]) | ||
| colnames(df_intensities) <- colnames(peakgroup_list)[col_idx] |
There was a problem hiding this comment.
You could use the select() function of dplyr. This also works with indices and then there is no need to set the colnames.
| #' @param row_index: Index of row (integer) | ||
| #' | ||
| #' @returns intensities_df_long: a dataframe with on each row a sample and their intensity | ||
| #' @returns intensities_df_long: Dataframe with on each row a sample and its intensity (matrix) |
There was a problem hiding this comment.
Is this variable a dataframe or a matrix?
| #' @param excel_workbook: An openxlsx Workbook object (workbook object) | ||
| #' @param sheetname: Name of the sheet where the plots are to be placed (string) | ||
| #' @param intensities_df: Dataframe containing intensities for controls and patients of a specific HMDB ID (matrix) | ||
| #' @param file_path: Filepath for the png (string) | ||
| #' @param hmdb_id: HMDB ID corresponding to intensities_df (string) | ||
| #' @param plot_width: Plot width for the png (integer) | ||
| #' @param col_width: Width of the column that has the plots (integer) | ||
| #' @param start_row_index: Index of the row where the plot has to be placed (integer) |
There was a problem hiding this comment.
The @param section is not consistent in this R file. For this function the data type is at the end, but previous functions have the datatype within the text.
| mz_range <- seq(70, 599, by = 1) | ||
| mz_missing <- setdiff(mz_range, mzmed_pgrp_ident) | ||
| # check if m/z are missing and make an .txt file with information | ||
| # check if m/z are missing and make a txt file with information |
There was a problem hiding this comment.
Does this function make a txt file?
| is_below_threshold <- is_selection_subset[below_threshold_index, ] | ||
| # add information on scan mode | ||
| is_below_threshold <- cbind(is_below_threshold, scanmode = rep(scanmode, nrow(is_below_threshold))) | ||
| if (nrow(is_below_threshold) > 0) { |
There was a problem hiding this comment.
The indentation is incorrect.
| #' @return df_zscores_to_helix: dataframe with patient data with only metabolites for Helix and violin plots | ||
| #' with Helix name, high/low Z-score cutoffs | ||
| #' with Helix name, high/low Z-score cutoffs |
There was a problem hiding this comment.
See previous comment on indentations for docstrings.
| #' @return top_metab_patient: Dataframe with metabolites that exceed the min and max Z-score cutoffs | ||
| #' for the selected patient (matrix) |
There was a problem hiding this comment.
See previous comment on indentations for docstrings.
| patient_id_sub <- patient_id | ||
| suffix <- "" | ||
| if (grepl("Diagnostics", pdf_dir) && is_diagnostic_patients(patient_id)) { | ||
| if (grepl("Diagnost", pdf_dir) && is_diagnostic_patients(patient_id)) { |
There was a problem hiding this comment.
Why is this changed to Diagnost?
|
|
||
| # set new column names and calculate Z-scores | ||
| colnames_zscores <- NULL | ||
| peakgroup_list_zscores <-peakgroup_list |
There was a problem hiding this comment.
Why do you create a new variable, peakgroup_list_zscores? Is it not possible to use 1 variable?
| observed_mz <- peakgroup_list$mzmed.pgrp[row_index] | ||
| theor_mz <- peakgroup_list$theormz_HMDB[row_index] | ||
| peakgroup_list$ppmdev[row_index] <- 10^6 * (observed_mz - theor_mz) / theor_mz | ||
| peakgroup_list_ppm <- peakgroup_list |
There was a problem hiding this comment.
Same as the previous comment, why do you create a new variable without changing the original variable?
2 participants
The R scripts for the DIMS pipeline (main scripts for every step of the pipeline and the function scripts in folders export and preprocessing) have been cleaned up:
In addition, the variable outdir is no longer necessary and has been removed. The output files which start with outlist* have been renamed peakgroup_list*.
The function seqToIntervals has been removed from peak_finding_functions.R since the package R.utils is incorporated in docker image v1.4 and an alternative function has been used for gaussfunc. All NF scripts now use the updated docker image.
Argument parsing is part of another feature branch and can be ignored in this PR. Also, GenerateExcel.R and generate_excel_functions.R have been refactored in feature DIMS_DrugDB. The log2 operation has been removed from generate_violin_plots_functions.R.