-
Notifications
You must be signed in to change notification settings - Fork 4
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
Short term change #94
base: devel
Are you sure you want to change the base?
Changes from all commits
234bc47
4e9568a
19937c7
e451d86
07e33b4
452a709
6755ca9
7501031
d0ac31f
99b98e3
9529271
b2afe4b
c34b0d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,30 @@ | ||
# Generated by roxygen2: do not edit by hand | ||
|
||
export(addBaselineDivergence) | ||
export(addShortTermChange) | ||
export(addStepwiseDivergence) | ||
export(getBaselineDivergence) | ||
export(getShortTermChange) | ||
export(getStepwiseDivergence) | ||
exportMethods(addBaselineDivergence) | ||
exportMethods(addShortTermChange) | ||
exportMethods(addStepwiseDivergence) | ||
exportMethods(getBaselineDivergence) | ||
exportMethods(getShortTermChange) | ||
exportMethods(getStepwiseDivergence) | ||
import(TreeSummarizedExperiment) | ||
import(mia) | ||
importFrom(SummarizedExperiment,colData) | ||
importFrom(dplyr,"%>%") | ||
importFrom(dplyr,arrange) | ||
importFrom(dplyr,as_tibble) | ||
importFrom(dplyr,group_by) | ||
importFrom(dplyr,lag) | ||
importFrom(dplyr,mutate) | ||
importFrom(dplyr,summarize) | ||
importFrom(dplyr,ungroup) | ||
importFrom(ggplot2,aes) | ||
importFrom(ggplot2,ggplot) | ||
importFrom(ggrepel,geom_text_repel) | ||
importFrom(mia,rarefyAssay) | ||
importFrom(mia,transformAssay) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
#' @title Short term Changes in Abundance | ||
#' | ||
#' @description Calculates short term changes in abundance of taxa | ||
#' using temporal Abundance data. | ||
#' | ||
#' @param x a \code{\link{SummarizedExperiment}} object. | ||
#' @param assay.type \code{Character scalar}. Specifies the name of assay | ||
#' used in calculation. (Default: \code{"counts"}) | ||
#' @param name \code{Character scalar}. Specifies a name for storing | ||
#' short term results. (Default: \code{"short_term_change"}) | ||
#' @param ... additional arguments. | ||
#' | ||
#' | ||
#' @return \code{getShortTermChange} returns \code{DataFrame} object containing | ||
#' the short term change in abundance over time for a microbe. | ||
#' \code{addShortTermChange}, on the other hand, returns a | ||
#' \code{\link[SummarizedExperiment:SummarizedExperiment-class]{SummarizedExperiment}} | ||
#' object with these results in its \code{metadata}. | ||
#' | ||
#' @details This approach is used by Wisnoski NI and colleagues | ||
#' \url{https://github.com/nwisnoski/ul-seedbank}. Their approach is based on | ||
#' the following calculation log(present abundance/past abundance). | ||
#' Also a compositional version using relative abundance similar to | ||
#' Brian WJi, Sheth R et al | ||
#' \url{https://www.nature.com/articles/s41564-020-0685-1} can be used. | ||
#' This approach is useful for identifying short term growth behaviors of taxa. | ||
Comment on lines
+21
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove this indentation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And also from above |
||
#' | ||
#' @name addShortTermChange | ||
#' | ||
#' | ||
#' @examples | ||
#' | ||
#' # Load time series data | ||
#' data(minimalgut) | ||
#' tse <- minimalgut | ||
#' | ||
#' short_time_labels <- c("74.5h", "173h", "438h", "434h", "390h") | ||
#' | ||
#' # Subset samples by Time_label and StudyIdentifier | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of this comment, say what it is the goal: get specified time points from certain bioreactor |
||
#' tse <- tse[, !(tse$Time_label %in% short_time_labels)] | ||
#' tse <- tse[, (tse$StudyIdentifier == "Bioreactor A")] | ||
#' | ||
#' # Get short term change | ||
#' # Case of rarefying counts | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not rarefy counts. Are the comments in wrong place? |
||
#' tse <- transformAssay(tse, method = "relabundance") | ||
#' getShortTermChange(tse, assay.type = "relabundance", time.col = "Time.hr") | ||
#' | ||
#' # Case of transforming counts | ||
#' tse <- rarefyAssay(tse, assay.type = "counts") | ||
#' getShortTermChange(tse, assay.type = "subsampled", time.col = "Time.hr") | ||
NULL | ||
|
||
#' @rdname addShortTermChange | ||
#' @export | ||
#' @importFrom dplyr arrange as_tibble summarize "%>%" | ||
#' @importFrom ggplot2 ggplot aes | ||
#' @importFrom ggrepel geom_text_repel | ||
#' @importFrom mia rarefyAssay transformAssay | ||
Comment on lines
+56
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are not used |
||
#' @importFrom SummarizedExperiment colData | ||
Comment on lines
+55
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add these above the function that utilizes these. Easier to remove when we do not need them anymore |
||
setMethod("getShortTermChange", signature = c(x = "SummarizedExperiment"), | ||
function(x, assay.type = "counts", ...){ | ||
Comment on lines
+60
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the function should also take into account the grouping, i.e., from which patient/bioreactor the data is coming |
||
############################## Input check ############################# | ||
# Check validity of object | ||
if (nrow(x) == 0L){ | ||
stop("No data available in `x` ('x' has nrow(x) == 0L.)", | ||
call. = FALSE) | ||
} | ||
# Check assay.type | ||
.check_assay_present(assay.type, x) | ||
########################### Growth Metrics ############################ | ||
grwt <- .calculate_growth_metrics(x, assay.type = assay.type, ...) | ||
# Clean and format growth metrics | ||
grs.all <- .clean_growth_metrics(grwt, ...) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if these are needed, as these are quite basic metric that can be easily calculated |
||
return(grs.all) | ||
} | ||
) | ||
|
||
#' @rdname addShortTermChange | ||
#' @export | ||
setMethod("addShortTermChange", signature = c(x = "SummarizedExperiment"), | ||
function(x, assay.type = "counts", name = "short_term_change", ...){ | ||
# Calculate short term change | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To support altexp, you could add .get_and_check_altExp() similarly to divergence functions |
||
res <- getShortTermChange(x, ...) | ||
Comment on lines
+79
to
+83
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check BiocHCeck::BiocHeck() at least indentation is off |
||
# Add to metadata | ||
x <- .add_values_to_metadata(x, name, res, ...) | ||
return(x) | ||
} | ||
) | ||
|
||
################################ HELP FUNCTIONS ################################ | ||
|
||
# wrapper to calculate growth matrix | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Start comments with capital |
||
.calculate_growth_metrics <- function(x, assay.type, time.col = NULL, ...) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. time.col is always needed? Then add it to exported function. Also it is not checked that it can be found from colData(). (meltSE checks that but the error message says about add.col) |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove this empty line |
||
# Reshape data and calculate growth metrics | ||
assay_data <- meltSE(x, assay.type = assay.type, | ||
add.col = time.col, row.name = "Feature_ID") | ||
assay_data <- assay_data %>% | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |> operators. They are used in elsewhere |
||
arrange( !!sym(time.col) ) %>% | ||
group_by(Feature_ID) %>% | ||
mutate( | ||
time_lag = !!sym(time.col) - lag( !!sym(time.col) ), | ||
growth_diff =!!sym(assay.type) - lag(!!sym(assay.type)), | ||
growth_rate = (!!sym(assay.type) - lag(!!sym(assay.type))) / lag(!!sym(assay.type)), | ||
var_abund = (!!sym(assay.type) - lag(!!sym(assay.type))) / time_lag | ||
) | ||
return(assay_data) | ||
Comment on lines
+99
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should take into account duplicated values. I tink you could give warning first and then try something like this
|
||
} | ||
|
||
.clean_growth_metrics <- function(grwt, time.col = NULL, ...) { | ||
# Calculate max growth | ||
maxgrs <- grwt %>% | ||
summarize(max_growth = max(growth_diff, na.rm = TRUE)) | ||
# Merge growth data with max growth | ||
grs.all <- merge(grwt, maxgrs, by = "Feature_ID") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for variables use grs_all. grs.all is reserved for parameters |
||
# Add 'ismax' column indicating if the growth is the maximum | ||
grs.all <- grs.all %>% | ||
mutate(ismax = ifelse(growth_diff == max_growth, TRUE, FALSE)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This column is in format "ismax" while others are in "is_max". Also it would be neater solution to just use |
||
# Clean and abbreviate FeatureID names | ||
grs.all$Feature_IDabb <- toupper(abbreviate(grs.all$Feature_ID, | ||
minlength = 3, | ||
method = "both.sides")) | ||
Comment on lines
+120
to
+122
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These feature names come directly from rownames()? Then I think they should be kept unchanged |
||
# Create 'Feature.time' column combining abbreviation and time information | ||
grs.all$Feature_time <- paste0(grs.all$Feature_IDabb, " ", | ||
grs.all[[time.col]], "h") | ||
Comment on lines
+123
to
+125
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this line needed? The info is already in the table. Moreover, the unit can be other than hours |
||
|
||
return(grs.all) | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
test_that("getShortTermChange", { | ||
library(SummarizedExperiment) | ||
# Load dataset | ||
data(minimalgut) | ||
tse <- minimalgut | ||
# Check if the function handles empty input | ||
empty_se <- SummarizedExperiment() | ||
expect_error(getShortTermChange(empty_se), | ||
"No data available in `x`") | ||
|
||
# Should still return a dataframe | ||
short_time_labels <- c("74.5h", "173h", "438h", "434h", "390h") | ||
# Subset samples by Time_label and StudyIdentifier | ||
tse_filtered <- tse[, !(tse$Time_label %in% short_time_labels)] | ||
tse_filtered <- tse_filtered[, (tse_filtered$StudyIdentifier == "Bioreactor A")] | ||
|
||
expect_true(all(!(tse_filtered$Time_label %in% short_time_labels))) | ||
|
||
result <- getShortTermChange(tse_filtered, time.col = "Time.hr") | ||
# Expected output is a dataframe | ||
expect_true(is.data.frame(result)) | ||
expect_true("growth_diff" %in% colnames(result)) | ||
# Test some expected properties (e.g., that growth_diff isn't all NAs) | ||
expect_false(all(is.na(result$growth_diff))) | ||
Comment on lines
+11
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The values should be also checked |
||
}) |
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.
You could use math notation, it would be easier to read. Also the calculated values include other measurements than this
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.
You could describe them also