Merge pull request #9550 from rharao/rharao-findallmarkers-groupby

Implement group.by arg to FindAllMarkers

DESCRIPTION

@@ -1,5 +1,5 @@
 Package: Seurat
-Version: 5.1.0.9017
+Version: 5.1.0.9018
 Title: Tools for Single Cell Genomics
 Description: A toolkit for quality control, analysis, and exploration of single cell RNA sequencing data. 'Seurat' aims to enable users to identify and interpret sources of heterogeneity from single cell transcriptomic measurements, and to integrate diverse types of single cell data. See Satija R, Farrell J, Gennert D, et al (2015) <doi:10.1038/nbt.3192>, Macosko E, Basu A, Satija R, et al (2015) <doi:10.1016/j.cell.2015.05.002>, Stuart T, Butler A, et al (2019) <doi:10.1016/j.cell.2019.05.031>, and Hao, Hao, et al (2020) <doi:10.1101/2020.10.12.335331> for more details.
 Authors@R: c(

NEWS.md

@@ -1,6 +1,8 @@
 # Unreleased
 
 ## Changes
+- Added `group.by` parameter to `FindAllMarkers`, allowing users to regroup their data using a non-default identity class prior to performing differential expression ([#9550](https://github.com/satijalab/seurat/pull/9550))
+#' performing differential expression (see example); \code{"ident"} to use Idents
 - Added `image.type` parameter to `Read10X_Image` enabling `VisiumV1` instances to be populated instead of instances of the default `VisiumV2` class ([#9556](https://github.com/satijalab/seurat/pull/9556))
 - Fixed `IntegrateLayers` to respect the `dims.to.integrate` parameter.
 - Added `stroke.size` parameter to `DimPlot` ([#8180](https://github.com/satijalab/seurat/pull/8180))

R/differential_expression.R

@@ -46,6 +46,7 @@ FindAllMarkers <- function(
   object,
   assay = NULL,
   features = NULL,
+  group.by = NULL,
   logfc.threshold = 0.1,
   test.use = 'wilcox',
   slot = 'data',
@@ -75,11 +76,25 @@ FindAllMarkers <- function(
     return.thresh <- 0.7
   }
   if (is.null(x = node)) {
+    if (!is.null(x = group.by) && !identical(x = group.by, y = "ident")) {
+      if (length(x = group.by) == 1 && ! group.by %in% colnames(x = object@meta.data)) {
+        stop("'", group.by, "' not found in object metadata")
+      }
+      Idents(object = object) <- group.by
+    }
     idents.all <- sort(x = unique(x = Idents(object = object)))
   } else {
     if (!PackageCheck('ape', error = FALSE)) {
       stop(cluster.ape, call. = FALSE)
     }
+    if (!is.null(group.by)) {
+      warning(
+        paste0(
+          "The `group.by` parameter for `FindAllMarkers` ",
+          "is ignored when `node` is set."
+        )
+      )
+    }
     tree <- Tool(object = object, slot = 'BuildClusterTree')
     if (is.null(x = tree)) {
       stop("Please run 'BuildClusterTree' before finding markers on nodes")
@@ -896,7 +911,7 @@ FindMarkers.DimReduc <- function(
 #' use all other cells for comparison; if an object of class \code{phylo} or
 #' 'clustertree' is passed to \code{ident.1}, must pass a node to find markers for
 #' @param group.by Regroup cells into a different identity class prior to 
-#' performing differential expression (see example)
+#' performing differential expression (see example); \code{"ident"} to use Idents
 #' @param subset.ident Subset a particular identity class prior to regrouping. 
 #' Only relevant if group.by is set (see example)
 #' @param assay Assay to use in differential expression testing
@@ -919,10 +934,13 @@ FindMarkers.Seurat <- function(
   reduction = NULL,
   ...
 ) {
-  if (!is.null(x = group.by)) {
+  if (!is.null(x = group.by) && !identical(x = group.by, y = "ident")) {
     if (!is.null(x = subset.ident)) {
       object <- subset(x = object, idents = subset.ident)
     }
+    if (length(x = group.by) == 1 && ! group.by %in% colnames(x = object@meta.data)) {
+      stop("'", group.by, "' not found in object metadata")
+    }
     Idents(object = object) <- group.by
   }
   if (!is.null(x = assay) && !is.null(x = reduction)) {

tests/testthat/test_differential_expression.R

@@ -366,12 +366,17 @@ test_that("BPCells FindMarkers gives same results", {
 
 # Tests for FindAllMarkers
 # -------------------------------------------------------------------------------
-results <- suppressMessages(suppressWarnings(FindAllMarkers(object = pbmc_small,pseudocount.use=1)))
-results.clr <- suppressMessages(suppressWarnings(FindAllMarkers(object = clr.obj, pseudocount.use=1)))
-results.sct <- suppressMessages(suppressWarnings(FindAllMarkers(object = sct.obj, pseudocount.use=1, vst.flavor = "v1")))
-results.pseudo <- suppressMessages(suppressWarnings(FindAllMarkers(object = pbmc_small, pseudocount.use = 0.1)))
 
 test_that("FindAllMarkers works as expected", {
+  pbmc_copy <- pbmc_small
+  Idents(pbmc_copy) <- "orig.ident"
+
+  results <- suppressMessages(suppressWarnings(FindAllMarkers(object = pbmc_small, pseudocount.use = 1)))
+  results.clr <- suppressMessages(suppressWarnings(FindAllMarkers(object = clr.obj, pseudocount.use = 1)))
+  results.sct <- suppressMessages(suppressWarnings(FindAllMarkers(object = sct.obj, pseudocount.use = 1, vst.flavor = "v1")))
+  results.pseudo <- suppressMessages(suppressWarnings(FindAllMarkers(object = pbmc_small, pseudocount.use = 0.1)))
+  results.gb <- suppressMessages(suppressWarnings(FindAllMarkers(object = pbmc_copy, pseudocount.use = 1, group.by = "RNA_snn_res.1")))
+
   expect_equal(colnames(x = results), c("p_val", "avg_log2FC", "pct.1", "pct.2", "p_val_adj", "cluster", "gene"))
   expect_equal(results[1, "p_val"], 9.572778e-13, tolerance = 1e-18)
   expect_equal(results[1, "avg_log2FC"], -6.030507, tolerance = 1e-6)
@@ -407,6 +412,10 @@ test_that("FindAllMarkers works as expected", {
   expect_equal(results.pseudo[1, "p_val_adj"], 2.201739e-10, tolerance = 1e-15)
   expect_equal(nrow(x = results.pseudo), 222)
   expect_equal(rownames(results.pseudo)[1], "HLA-DPB1")
+
+  # Setting `group.by` the group by parameter is equivalent
+  # to setting the object's `Idents` before running `FindAllMarkers`.
+  expect_equal(results.gb, results)
 })