Skip to content
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

Open
wants to merge 13 commits into
base: devel
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,20 @@ biocViews: Microbiome, Software, Sequencing
License: Artistic-2.0 | file LICENSE
Depends:
R (>= 4.4.0),
mia
ggplot2,
mia,
SummarizedExperiment
Imports:
dplyr,
methods,
S4Vectors,
SingleCellExperiment,
SummarizedExperiment,
TreeSummarizedExperiment
TreeSummarizedExperiment,
ggrepel,
reshape2
Suggests:
BiocStyle,
devtools,
ggplot2,
knitr,
lubridate,
miaViz,
Expand Down
13 changes: 13 additions & 0 deletions NAMESPACE
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)
12 changes: 12 additions & 0 deletions R/AllGenerics.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,15 @@ setGeneric("getStepwiseDivergence", signature = c("x"), function(x, ...)
#' @export
setGeneric("addStepwiseDivergence", signature = "x", function(x, ...)
standardGeneric("addStepwiseDivergence"))

#' @rdname addShortTermChange
#' @export
setGeneric("addShortTermChange", signature = "x", function(x, ...)
standardGeneric("addShortTermChange"))

#' @rdname addShortTermChange
#' @export
setGeneric("getShortTermChange", signature = "x", function(x, ...)
standardGeneric("getShortTermChange"))


128 changes: 128 additions & 0 deletions R/getShortTermChange.R
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
Copy link
Collaborator

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

Copy link
Collaborator

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

#' \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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this indentation

Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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, ...)
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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, ...) {
Copy link
Collaborator

Choose a reason for hiding this comment

The 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)


Copy link
Collaborator

Choose a reason for hiding this comment

The 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 %>%
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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

df <- meltSE(tse, add.col = TRUE)
df |>
    group_by(StudyIdentifier, FeatureID, Time.hr) |>
    mutate(mean_abundance_for_timepoint = mean(counts, na.rm = TRUE)) |>
    distinct(StudyIdentifier, FeatureID, Time.hr, .keep_all = TRUE) |>
    ungroup() |>
    arrange(StudyIdentifier, FeatureID, Time.hr) |>
    group_by(StudyIdentifier, FeatureID) |>
    mutate(growth_diff = mean_abundance_for_timepoint - lag(mean_abundance_for_timepoint)) |>
    ungroup()

}

.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")
Copy link
Collaborator

Choose a reason for hiding this comment

The 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))
Copy link
Collaborator

Choose a reason for hiding this comment

The 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 growth_diff == max_growth as it already returns boolean vector

# 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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)
}
1 change: 1 addition & 0 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,4 @@
.check_assay_present <- mia:::.check_assay_present
.add_values_to_colData <- mia:::.add_values_to_colData
.check_and_get_altExp <- mia:::.check_and_get_altExp
.add_values_to_metadata <- mia:::.add_values_to_metadata
69 changes: 69 additions & 0 deletions man/addShortTermChange.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions man/miaTime-package.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions tests/testthat/test-getShortTermChange.R
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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The values should be also checked

})
Loading