-
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
Preprocess #469
Preprocess #469
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.
Thanks, aside from the comment on the filtering looks good. If you think we really want the filtering apart from the other ones, I would approve, otherwise we need to slightly change the setup
return(list( | ||
data = tmm_proccessing(data, omic_type) | ||
)) | ||
} |
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.
Error: Error in DGEList(counts = assay(data)): could not find function "DGEList"
I assume we need to add a package to the renv?
print(paste0("Remove anything of rowCount <=",filter_threshold_samplewise," in at least ",filter_samplesize," samples")) | ||
return(data[which(rowSums(assay(data) > filter_threshold_samplewise) >= filter_samplesize),]) | ||
} | ||
} |
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.
I honestly thought we would replace the "prefiltering" option and instead always
have the option of prefiltering. I think this would make sense as filtering is not entirely but partially independent of the user.
data <- prefiltering(data, omic_type) | ||
# Create DGEList object | ||
dge <- DGEList(counts = assay(data)) | ||
# TODO visulaize norn factor distribution |
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.
Is this a necessary todo still? We could think about putting this on top of the function since in the future, comments in out functions will be added to util in code generation.
)%>% helper(type = "markdown", content = "PreProcessing_Procedures"), | ||
|
||
# Second dropdown: Options based on Processing Type | ||
uiOutput(outputId = "dynamic_options_ui"), |
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.
We can do it like this, i have however noticed faster performance with UI elements directly defined in the UI through conditional panels. This is something we should definitely do at some point more rigorously but for review optional
"<br>",ifelse(any(as.data.frame(assay(data)) < 0),"Be aware that processed data has negative values, hence no log fold changes can be calculated","")) | ||
"<br>",ifelse(input$processing_type == "Log-Based","In case of 0's present logX(data+1) is done",""), | ||
"<br>","See help for details", | ||
"<br>",ifelse(any(as.data.frame(assay(data)) < 0),"Be aware that processed data has negative values!",""), ## IS THAT TRUE?? |
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.
from what i can see in the violin plot: yes, i think the comment can be removed
filter_threshold = filter_threshold, | ||
filter_threshold_samplewise = filter_threshold_samplewise, | ||
filter_samplesize = filter_samplesize, | ||
limma_intercept = limma_intercept, | ||
limma_formula = limma_formula | ||
) |
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.
these need to be added to par_tmp for pre_processing to work
# Conflicts: # program/shinyApp/R/pre_processing/util.R # program/shinyApp/server.R
addressing #429
Opened #468 for main documentaion update incl. desicion tree which is still missing