From abae731c34e0b9deeb89e8e012f2db632672f776 Mon Sep 17 00:00:00 2001
From: vkozareva <velina.kozareva@gmail.com>
Date: Wed, 5 Jun 2019 13:09:40 -0400
Subject: [PATCH] Fix plotFeature to shuffle cells correctly

Also make sure plots are returned in same order as datasets.
---
 R/liger.R                                | 19 ++++++++++---------
 tests/testthat/test_post_factorization.R |  3 +++
 tests/testthat/test_preprocessing.R      |  1 +
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/R/liger.R b/R/liger.R
index f9e0dd13..a435dc6d 100644
--- a/R/liger.R
+++ b/R/liger.R
@@ -2342,17 +2342,17 @@ plotFeature <- function(object, feature, by.dataset = T, title = NULL, pt.size =
   } else {
     discrete <- TRUE
   }
+  if (by.dataset) {
+    dr_df$dataset <- object@cell.data$dataset
+  } else {
+    dr_df$dataset <- factor("single")
+  }
   if (do.shuffle) {
     set.seed(rand.seed)
     idx <- sample(1:nrow(dr_df))
     dr_df <- dr_df[idx, ]
   }
   p_list <- list()
-  if (by.dataset) {
-    dr_df$dataset <- object@cell.data$dataset
-  } else {
-    dr_df$dataset <- factor("single")
-  }
   for (sub_df in split(dr_df, f = dr_df$dataset)) {
     ggp <- ggplot(sub_df, aes(x = dr1, y = dr2, color = feature)) + geom_point(size = pt.size) 
     
@@ -2384,17 +2384,18 @@ plotFeature <- function(object, feature, by.dataset = T, title = NULL, pt.size =
       ggp <- ggp + theme(legend.position = "none")
     }
     p_list[[as.character(sub_df$dataset[1])]] <- ggp
-    
-    if (!return.plots) {
-      print(ggp)
-    }
   }
+  p_list <- p_list[names(object@raw.data)]
   if (return.plots){
     if (length(p_list) == 1) {
       return(p_list[[1]])
     } else {
       return(p_list)
     }
+  } else {
+    for (plot in p_list) {
+      print(plot)
+    }
   }
 }
 
diff --git a/tests/testthat/test_post_factorization.R b/tests/testthat/test_post_factorization.R
index 4dc41491..4b816572 100644
--- a/tests/testthat/test_post_factorization.R
+++ b/tests/testthat/test_post_factorization.R
@@ -131,6 +131,9 @@ plotfeatures_plots <- plotFeature(ligex, feature = 'nUMI', by.dataset = T, retur
 test_that("plotFeature returns correct ggplot objects", {
   expect_equal(length(plotfeatures_plots), length(ligex@raw.data))
   expect_is(plotfeatures_plots[[1]], class = c("ggplot"))
+  expect_equal(rownames(plotfeatures_plots[[1]]$data)[1:5], 
+               c("AATGCGTGGCTATG", "GAAAGATGATTTCC", "TTCCAAACTCCCAC", "CACTGAGACAGTCA",
+                 "GACGGCACACGGGA"))
 })
 
 # Tests for subsetting, object conversion
diff --git a/tests/testthat/test_preprocessing.R b/tests/testthat/test_preprocessing.R
index 6c7d6850..c3fd13cd 100644
--- a/tests/testthat/test_preprocessing.R
+++ b/tests/testthat/test_preprocessing.R
@@ -23,6 +23,7 @@ test_that("Sparse matrices created", {
   expect_is(ligex@raw.data[[1]], "CsparseMatrix")
 })
 
+# note that seqwell data is previously normalized, so nUMI is 10000 for all cells
 test_that("cell.data created correctly", {
   expect_is(ligex@cell.data, "data.frame")
   expect_equal(rownames(ligex@cell.data)[1:10], colnames(ligex@raw.data[[1]])[1:10])