statistical tests - pairwise - omnibus #757

0xMuluh · 2025-08-01T05:52:48Z

#733

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

antagomir

Is the name getPairwiseTest correct with Kruskal test? Kruskal is not a pairwise test.

In examples:

tse <- tse[1:100, ]
tse <- agglomerateByRank(tse, rank = "Genus")

-> could we just agglomerate to higher rank (e.g. Phylum) for a quick example? Then no need to take subset of 100.

man/getPairwiseTest.Rd

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

tests/testthat/test-getPairwiseTest.R

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

0xMuluh · 2025-08-06T17:41:56Z

Is the name getPairwiseTest correct with Kruskal test? Kruskal is not a pairwise test.

True. However, kruskal is folloed by dunn's test. For the test to be valid, user has to check that kruskal test is useful.

What other name is suitable getSignificance? getTest?

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

antagomir · 2025-08-06T19:30:24Z

Is the name getPairwiseTest correct with Kruskal test? Kruskal is not a pairwise test.

True. However, kruskal is folloed by dunn's test. For the test to be valid, user has to check that kruskal test is useful.

What other name is suitable getSignificance? getTest?

We might like to distinguish between these two cases:

Pairwise test between two groups (e.g. Wilcoxon test)
Multi-group test (e.g. Kruskal-Wallis test), followed by a post-hoc test (e.g. Dunn). Potentially we would even like to split these into two different functions that could be used subsequently.

The (2) could reduce to (1) if there are only 2 groups but it is still conceptually different.

These are very basic methods and one could question whether it makes sense to implement new wrappers for them.

The motivation would be that this helps to standardize the way this is done with TreeSE objects, and make it faster to deploy standard summary tables and figures with minimal extra effort. Then also new improved methods can be proposed using the same logic (e.g. probabilistic variants).

I would start by implementing (1) as getPairwiseTest.

Once that works, we could tackle (2). That could go in its own issue? You can try to implement these in parallel.

0xMuluh · 2025-08-07T07:12:28Z

Thanks for raising these points. Yes, the distinction between pairwise and multi-group testing should be made clear. I agree that getPairwiseTest is not the most accurate name when Kruskal-Wallis is involved.

1. Pairwise test between two groups (e.g. Wilcoxon test)

I'll proceed with refactoring (1) as getPairwiseTest . This case is currently covered for un/paired. Additionally there is also support for t -test. It could be dropped since the interests is towards non parametric tests ( or maybe issue warning if data violates assumptions?)

2. Multi-group test (e.g. Kruskal-Wallis test), followed by a post-hoc test (e.g. Dunn). Potentially we would even like to split these into two different functions that could be used subsequently.

Right, for the omnibus tests, this can be easily separated though I would question here if there is use case where e.g Kruskal-Wallis test is not followed by e.g Dunn's test? Hence the need why it is automated.

The (2) could reduce to (1) if there are only 2 groups but it is still conceptually different.

True, a simple check can deduce if the number of groups and assign the potential test, but i wonder if we should decide this for user as it would be too opinionated.

Currently the supported methods(with effect sizes, means and log2FCs) include;

Wilcoxon → un/paired
t-test → un/paired
Kruskal → followed by Dunn
Friedman → followed by Wilcoxon

The tibble returned by the current fxn is the same for all except for the global tests where the attrib contains omnibus tests.

The current fxn is biased as it runs all these tests without check/warning user if test is suitable(non-normality, small n, number of groups... ) but again I would expect user to know their design/experiment.

The image shows an example return.

Should fxn determine test based on user data? or perhaps issue warnings or stop completely ?
Should post-hoc be optional?

As concerns the naming, this can be addressed by splitting omnibus/post-hoc logic from non/pairwise comparisons:

getOmnibusTest() → Kruskal/Friedman.
getPosthocTest() → Dunn/Wilcoxon-signed-rank.
allow getTest() to chain them (default: post-hoc=TRUE).

Enough examples can demonstrate use cases.
If these address the aforementioned concerns, then I can refactor the fxn.

antagomir · 2025-08-07T07:25:41Z

Separate the pairwise and omnibus tests, the user can decide which one they want to run and we don't decide for the usre.
Let's not check model assumptions as part of this, users are supposed to know/check their data separately.
Use case for omnibus without posthoc is for instance when omnibus does not reveal any significant differences -> no use to run posthoc. And in some cases it might be just interesting to make a quick test if differences exist regardless of where.
Consider if we could include results from multiple tests; like with alpha diversity the user can choose to get multiple indices, perhaps with this the user could choose to see p-values from multiple methods (e.g. Wilcoxon test and t-test p-values provided in the same table). If this gets too complicated then one test is enough.
Open a separate issue about omnibus tests to keep this more manageable.
Perhaps getPairwiseTest is a good name but these are DAA methods, intuitive name could also be something like getPairwiseDA
Internally we could have separate functions getWilcoxon, getttest etc.?
Hide from output table: "statistic", "magnitude"; put effsize and log2FC next to each other

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

0xMuluh · 2025-08-08T09:52:48Z

ping #759

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

0xMuluh · 2025-08-12T10:53:46Z

method is now updated to handle pairwise, omnibus and posthoc

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

TuomasBorman

I think the function structure can still be simplified and made more generic. I come back to this later.

This functionality is related to little bit bigger project. Let's discuss this in the office. This PR is good foundation for it.

TuomasBorman · 2025-08-24T04:37:06Z

R/getDA.R

+        # Create grouping to test these groups separately
+        dplyr::group_split(across(all_of(grouping_vars))) |>
+        purrr::map_df(function(df_group) {
+            tryCatch({


Avoid try-catch

TuomasBorman · 2025-08-24T04:37:22Z

R/getDA.R

+    pw <- df |>
+        as.data.frame() |>
+        # Create grouping to test these groups separately
+        dplyr::group_split(across(all_of(grouping_vars))) |>


Use importFrom instead

TuomasBorman · 2025-08-24T04:37:47Z

R/getDA.R

+.calc_DA <- function(
+    df, y, group, facet.by, pair.by, comp.by, features, 
+    da.method, p.adjust.method = "fdr", 
+    paired = !is.null(pair.by), include.effect = TRUE, 
+    digits = 3, ...
+) {
+    # Basic validation
+    if (!.is_a_bool(paired)) {
+        stop("'paired' must be TRUE or FALSE.", call. = FALSE)
+    }
+
+    if ( !.is_a_string(p.adjust.method) ) {
+        stop("'p.adjust.method' must be a character string.", call. = FALSE)
+    }
+
+    if ( !.is_an_integer(digits) ) {
+        stop("'digits' must be a single integer value.", call. = FALSE)
+    }
+    # Get variables. group will specify the grouping variables, i.e.,
+    # we test the significance for these groups separately.
+    grouping_vars <- c(facet.by, group)
+    # comp.by specify groups for comparison. If they are not
+    # specified, we make comparison between group.
+    comparison_vars <- c(comp.by) |> unique()
+    # grouping_vars: used to split data into groups for repeated tests
+    # comparison_vars: the variable(s) we are testing across
+    if( is.null(comparison_vars) ){
+        comparison_vars <- group
+    }
+    grouping_vars <- grouping_vars[ !grouping_vars %in% comparison_vars ]
+
+    formula <- as.formula(paste(y, "~", paste(comparison_vars, collapse = "+")))
+
+    res <-  global <- effect <- NULL
+    method_funs <- list(


Use same code-styling as elsewhere

TuomasBorman · 2025-08-24T04:40:49Z

R/getDA.R

+#' @rdname getDA
+#' @export
+setMethod("getDA", signature(x = "SummarizedExperiment"),
+    function(x, da.type = c("pairwise", "omnibus", "posthoc"), ...) {
+        da.type <- match.arg(da.type)
+        FUN <- switch(
+            da.type,
+            pairwise = getPairwiseDA,
+            omnibus  = getOmnibusDA,
+            posthoc  = getPosthocDA)
+        FUN(x, ...)
+    }
+)
+
+#' @rdname getDA
+#' @export
+setMethod("addDA", signature(x = "SummarizedExperiment"),
+    function(x, da.type = c("pairwise", "omnibus", "posthoc"),
+           name = "DAtest", ...) {
+        da.type <- match.arg(da.type)
+        res <- getDA(x, da.type, ...)
+        x <- .add_values_to_metadata(x, name, res)
+        return(x)
+    }
+)
+


I think we can remove these wrappers. They do not add much value

TuomasBorman · 2025-08-24T04:41:22Z

R/getDA.R

+#' @export
+setMethod("getPairwiseDA", signature(x = "SummarizedExperiment"), 
+    function(x, assay.type = NULL, features = NULL, row.var = NULL, 
+            col.var = NULL, group, facet.by = NULL, comp.by = NULL,


What is facet.by?

TuomasBorman · 2025-08-24T04:51:28Z

R/getDA.R

+    if (!is.null(pair.by)) {
+        coldata <- as.data.frame(colData(tse))
+
+        sample_counts <- coldata %>%
+            group_by(.data[[pair.by]]) %>%
+            summarise(n_samples = n(), .groups = "drop")
+
+        if (length(unique(sample_counts$n_samples)) > 1) {
+            stop(
+                "When 'pair.by' is specified, all subjects must have the ",
+                "same number of samples.\nRemove 'pair.by' or filter your ",
+                "data to include only subjects with balanced repeats.",
+                call. = FALSE
+            )
+        }
+    }


Is this required for the function to work? I guess user should already be aware of this, and we should not restrict user to run the method if some samples are missing from certain time points

TuomasBorman · 2025-12-08T07:46:15Z

These will be implemented in microbiome/daa#8

0xMuluh added 5 commits July 31, 2025 20:47

wip: pairwisetest

a494688

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

wip- pairwise tests

a1763d6

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

wip- test

a87127d

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

wip - test

c35f05f

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

Merge branch 'devel' into pt

afffa01

0xMuluh requested a review from antagomir August 1, 2025 05:52

0xMuluh added 3 commits August 1, 2025 09:23

rstatix

b371005

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

wip- load dplyr

fbaa5b4

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

wip- typo

3425fd3

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

antagomir reviewed Aug 1, 2025

View reviewed changes

man/getPairwiseTest.Rd Outdated Show resolved Hide resolved

up examples

8a5fbbd

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

antagomir reviewed Aug 4, 2025

View reviewed changes

tests/testthat/test-getPairwiseTest.R Outdated Show resolved Hide resolved

up

e34e529

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

up

6a6e6c4

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

0xMuluh changed the title ~~non parametric test wrapper~~ non-parametric tests - pairwise Aug 7, 2025

0xMuluh mentioned this pull request Aug 7, 2025

omnibus test wrapper #759

Open

wip getDA

23de33c

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

up

55e2953

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

0xMuluh changed the title ~~non-parametric tests - pairwise~~ statistical tests - pairwise - omnibus Aug 12, 2025

up

8d5972e

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

up

5603de2

Signed-off-by: Daena Rys <rysdaena8@gmail.com>

0xMuluh requested a review from TuomasBorman August 22, 2025 07:30

TuomasBorman requested changes Aug 24, 2025

View reviewed changes

Merge branch 'devel' into pt

fd4bb4b

TuomasBorman mentioned this pull request Dec 8, 2025

Take into account discussion elsewhere microbiome/daa#8

Open

TuomasBorman closed this Dec 8, 2025

statistical tests - pairwise - omnibus #757

statistical tests - pairwise - omnibus #757

Uh oh!

Conversation

0xMuluh commented Aug 1, 2025

Uh oh!

antagomir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

0xMuluh commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antagomir commented Aug 6, 2025

Uh oh!

0xMuluh commented Aug 7, 2025

Uh oh!

antagomir commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xMuluh commented Aug 8, 2025

Uh oh!

0xMuluh commented Aug 12, 2025

Uh oh!

TuomasBorman left a comment

Choose a reason for hiding this comment

Uh oh!

TuomasBorman Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

TuomasBorman Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

TuomasBorman Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

TuomasBorman Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

TuomasBorman Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

TuomasBorman Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

TuomasBorman commented Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

0xMuluh commented Aug 6, 2025 •

edited

Loading

antagomir commented Aug 7, 2025 •

edited

Loading