From d1c8824a28531182d6b0f17e39dd6a499399dd7c Mon Sep 17 00:00:00 2001
From: Will Townes <will.townes@gmail.com>
Date: Wed, 26 Aug 2020 18:53:15 -0400
Subject: [PATCH] simplify GLMPCA method definitions and set default assay to
 'counts' addresses #5 . Moves the L argument in front of assay since it is a
 required positional argument by glmpca

---
 NAMESPACE                    |  2 --
 R/AllGenerics.R              |  3 +--
 R/glmpca.R                   | 35 ++++++++++++++---------------------
 man/GLMPCA.Rd                | 16 ++++++++--------
 tests/testthat/test-glmpca.R |  2 +-
 5 files changed, 24 insertions(+), 34 deletions(-)

diff --git a/NAMESPACE b/NAMESPACE
index 9b7f0b0..b5ce16f 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -7,13 +7,11 @@ export(nullResiduals)
 exportMethods(GLMPCA)
 exportMethods(devianceFeatureSelection)
 exportMethods(nullResiduals)
-import(glmpca)
 importFrom(Matrix,t)
 importFrom(SingleCellExperiment,"reducedDim<-")
 importFrom(SummarizedExperiment,"assay<-")
 importFrom(SummarizedExperiment,"rowData<-")
 importFrom(SummarizedExperiment,assay)
 importFrom(SummarizedExperiment,rowData)
-importFrom(glmpca,glmpca)
 importFrom(methods,as)
 importFrom(methods,is)
diff --git a/R/AllGenerics.R b/R/AllGenerics.R
index c16dae3..73532f2 100644
--- a/R/AllGenerics.R
+++ b/R/AllGenerics.R
@@ -4,7 +4,7 @@
 #'   methods.
 #' @export
 setGeneric(
-    name = "devianceFeatureSelection",     #signature = 'object',
+    name = "devianceFeatureSelection", 
     def = function(object, ...) {
         standardGeneric("devianceFeatureSelection")
     }
@@ -18,7 +18,6 @@ setGeneric(
 #' @export
 setGeneric(
     name = "GLMPCA",
-    signature = 'object',
     def = function(object, ...) {
         standardGeneric("GLMPCA")
     }
diff --git a/R/glmpca.R b/R/glmpca.R
index bbd3ccb..4e14537 100644
--- a/R/glmpca.R
+++ b/R/glmpca.R
@@ -5,11 +5,11 @@
 #'   \code{\link[glmpca]{glmpca}}.
 #' 
 #' @param object A \code{\link{SingleCellExperiment}} or
-#'   \link{SummarizedExperiment} object. Alternatively, a matrix of integer
-#'   counts.
-#' @param assay a character or integer specifying which assay to use for GLM-PCA
-#'   (default = 1). Ignored if \code{object} is a matrix.
+#'   \link{SummarizedExperiment} object. Alternatively, a matrix-like object
+#'   of non-negative integer counts (such as a sparse \code{\link{Matrix}}).
 #' @param L the desired number of latent dimensions (integer).
+#' @param assay a character or integer specifying which assay to use for GLM-PCA
+#'   (default = 'counts'). Ignored if \code{object} is a matrix.
 #' @param ... further arguments passed to \code{\link[glmpca]{glmpca}}
 #'
 #' @return The original \code{SingleCellExperiment} or
@@ -26,14 +26,12 @@
 #' sce <- SingleCellExperiment::SingleCellExperiment(assays=list(counts=u))
 #' GLMPCA(sce, L = 2)
 #' 
-#' @import glmpca
 #' @importFrom SingleCellExperiment reducedDim<-
 #' @importFrom methods is
 #' @export
-setMethod(f = "GLMPCA",
-          signature = signature(object = "SummarizedExperiment"),
-          definition = function(object, assay = 1, L, ...){
-              res <- glmpca(Y = assay(object, assay), L = L, ...)
+setMethod(f = "GLMPCA","SummarizedExperiment",
+          definition = function(object, L, assay = "counts", ...){
+              res <- glmpca::glmpca(Y = assay(object, assay), L = L, ...)
               if(is(object, 'SingleCellExperiment')){
                   reducedDim(object, "GLMPCA") <- res$factors
               }
@@ -41,20 +39,15 @@ setMethod(f = "GLMPCA",
               return(object)
           })
 
+#this function is only needed because the glmpca signature has "Y" not "object"
+glmpca_wrapper<-function(object, L, ...){
+    glmpca::glmpca(object, L, ...)
+}
+
 #' @rdname GLMPCA
-#' @importFrom glmpca glmpca
 #' @export
-setMethod(f = "GLMPCA",
-          signature = signature(object = "matrix"),
-          definition = function(object, assay = 1, L, ...){
-              glmpca(Y = object, L = L, ...)
-          })
+setMethod(f = "GLMPCA", "matrix", definition=glmpca_wrapper)
 
 #' @rdname GLMPCA
-#' @importFrom glmpca glmpca
 #' @export
-setMethod(f = "GLMPCA",
-          signature = signature(object = "Matrix"),
-          definition = function(object, assay = 1, L, ...){
-              glmpca(Y = object, L = L, ...)
-          })
+setMethod(f = "GLMPCA", "Matrix", definition=glmpca_wrapper)
diff --git a/man/GLMPCA.Rd b/man/GLMPCA.Rd
index dc70ffc..f86459b 100644
--- a/man/GLMPCA.Rd
+++ b/man/GLMPCA.Rd
@@ -10,23 +10,23 @@
 \usage{
 GLMPCA(object, ...)
 
-\S4method{GLMPCA}{SummarizedExperiment}(object, assay = 1, L, ...)
+\S4method{GLMPCA}{SummarizedExperiment}(object, L, assay = "counts", ...)
 
-\S4method{GLMPCA}{matrix}(object, assay = 1, L, ...)
+\S4method{GLMPCA}{matrix}(object, L, ...)
 
-\S4method{GLMPCA}{Matrix}(object, assay = 1, L, ...)
+\S4method{GLMPCA}{Matrix}(object, L, ...)
 }
 \arguments{
 \item{object}{A \code{\link{SingleCellExperiment}} or
-\link{SummarizedExperiment} object. Alternatively, a matrix of integer
-counts.}
+\link{SummarizedExperiment} object. Alternatively, a matrix-like object
+of non-negative integer counts (such as a sparse \code{\link{Matrix}}).}
 
 \item{...}{further arguments passed to \code{\link[glmpca]{glmpca}}}
 
-\item{assay}{a character or integer specifying which assay to use for GLM-PCA
-(default = 1). Ignored if \code{object} is a matrix.}
-
 \item{L}{the desired number of latent dimensions (integer).}
+
+\item{assay}{a character or integer specifying which assay to use for GLM-PCA
+(default = 'counts'). Ignored if \code{object} is a matrix.}
 }
 \value{
 The original \code{SingleCellExperiment} or
diff --git a/tests/testthat/test-glmpca.R b/tests/testthat/test-glmpca.R
index dfdd3dd..eaab194 100644
--- a/tests/testthat/test-glmpca.R
+++ b/tests/testthat/test-glmpca.R
@@ -1,4 +1,5 @@
 context("Test GLMPCA")
+require(SingleCellExperiment)
 set.seed(1234)
 
 test_that("GLMPCA works", {
@@ -6,7 +7,6 @@ test_that("GLMPCA works", {
     u <- matrix(rpois(2000, 5), ncol=ncells)
     v <- log2(u + 1)
     
-    require(SingleCellExperiment)
     se <- SummarizedExperiment(assays=list(counts=u, logcounts=v))
     sce <- SingleCellExperiment(assays=list(counts=u, logcounts=v))