Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
We did this recently in minfi and getting acceptable performance was not trivial. We are still sorting out issues; we will eventually write about our experiences. Pete is working on some blog posts on this and we hope to do something more formal later. We would all love experience from more people on this, but just be aware that right now it involves pain. Best, Kasper On Wed, May 2, 2018 at 2:13 AM, Elizabeth Purdom wrote: > Thanks Hervé and Stephanie for your suggestions. I am really looking for a > S4 methods solution however, given how my package is already set up. Also, > I have several functions that I need to adapt in this way, so it seems > cleaner and simpler to do the class union, which sounds like is not a > problem — for Stephanie’s solution, for each function I’d have to write 2 > S4methods and an internal function which feels more cluttered code to > maintain for me. And it sounds like there’s not a virtual class I could use > instead so I am correct to defining it myself. > > In terms of the setClassUnion, I chose “DelayedArray” because I wanted to > capture HDF5Matrix and DelayedMatrix, but I now see that HDF5Matrix > inherits from DelayedMatrix. I must have missed that somehow. > > Thanks, > Elizabeth > > > On Apr 30, 2018, at 8:35 PM, Hervé Pagès wrote: > > > > The class union should probably be: > > > > setClassUnion("matrixOrDelayed", c("matrix", "DelayedMatrix")) > > > > i.e. use DelayedMatrix instead of DelayedArray. > > > > So in addition to the class union and to Stephanie's solution, which > > IMO are both valid solutions, you could also go for something like this: > > > > myNewRowMeans <- function(x,...) > > { > >if (length(dim(x)) != 2) > >stop("'x' must be a matrix-like object") > >... > > ) > > > > that is, just a regular function that checks that 'x' is matrix-like > > based on its number of dimensions. If you really want to restrict to > > matrix and DelayedMatrix only, replace the test with > > > >if (!(is.matrix(x) || is(x, "DelayedMatrix"))) > >stop("'x' must be a matrix or DelayedMatrix object") > > > > The difference being that now the function will reject matrix-like > > objects that are not matrix or DelayedMatrix objects (e.g. a Matrix > > derivative from the Matrix package). > > > > Cheers, > > H. > > > > > > On 04/30/2018 09:29 AM, Stephanie M. Gogarten wrote: > >> Rather than a class union, how about an internal function that is > called by the methods for both matrix and DelayedArray: > >> setGeneric("myNewRowMeans", function(x,...) { standardGeneric(" > myNewRowMeans")}) > >> #' @importFrom DelayedArray rowMeans > >> .myNewRowMeans <- function(x,...){ > >> # a lot of code independent of x > >> print("This is a lot of code shared regardless of class of x\n") > >> # a lot of code that depends on x, but is dispatched by the > functions called > >> out<-rowMeans(x) > >> #a lot of code based on output of out > >> out<-out+1 > >> return(out) > >> } > >> setMethod("myNewRowMeans", > >> signature = "matrix", > >> definition = function(x,...){ > >> .myNewRowMeans(x,...) > >> } > >> ) > >> setMethod("myNewRowMeans", > >> signature = "DelayedArray", > >> definition = function(x,...){ > >> .myNewRowMeans(x,...) > >> } > >> ) > >> On 4/30/18 9:10 AM, Tim Triche, Jr. wrote: > >>> But if you merge methods like that, the error method can be that much > more > >>> difficult to identify. It took a couple of weeks to chase that bug down > >>> properly, and it ended up down to rowMeans2 vs rowMeans. > >>> > >>> I suppose the merged/abstracted method allows to centralize any such > >>> dispatch into one place and swap out ill-behaved methods once > identified, > >>> so as long as DelayedArray/DelayedMatrixStats quirks are > >>> documented/understood, maybe it is better to create this union class? > >>> > >>> The Matrix/matrixStats/DelayedMatrix/DelayedMatrixStats situation has > been > >>> "interesting" in practical terms, as seemingly simple abstractions > appear > >>> to require more thought. That was my only point. > >>> > >>> > >>> --t > >>> > >>> On Mon, Apr 30, 2018 at 11:28 AM, Martin Morgan < > >>> martin.mor...@roswellpark.org> wrote: > >>> > But that issue will be fixed, so Tim's advice is inappropriate. > > > On 04/30/2018 10:42 AM, Tim Triche, Jr. wrote: > > > Don't do that. Seriously, just don't. > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__github. > com_Bioconductor_DelayedArray_issues_16&d=DwIDaQ&c= > eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m= > Rhy4i6H9xaY8HzWv9v_jhOnp5OyEpJcG52RP3nHorU8&s= > olbErqY3_l7i45-WeTkaUNGalrQQr-7i59rhJVF6OGQ&e= > > > > --t > > > > On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < > > epur...@stat.berkeley.edu> wrote: > > > > Hello, > >> > >> I am trying to extend my packa
Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
Thanks Hervé and Stephanie for your suggestions. I am really looking for a S4 methods solution however, given how my package is already set up. Also, I have several functions that I need to adapt in this way, so it seems cleaner and simpler to do the class union, which sounds like is not a problem — for Stephanie’s solution, for each function I’d have to write 2 S4methods and an internal function which feels more cluttered code to maintain for me. And it sounds like there’s not a virtual class I could use instead so I am correct to defining it myself. In terms of the setClassUnion, I chose “DelayedArray” because I wanted to capture HDF5Matrix and DelayedMatrix, but I now see that HDF5Matrix inherits from DelayedMatrix. I must have missed that somehow. Thanks, Elizabeth > On Apr 30, 2018, at 8:35 PM, Hervé Pagès wrote: > > The class union should probably be: > > setClassUnion("matrixOrDelayed", c("matrix", "DelayedMatrix")) > > i.e. use DelayedMatrix instead of DelayedArray. > > So in addition to the class union and to Stephanie's solution, which > IMO are both valid solutions, you could also go for something like this: > > myNewRowMeans <- function(x,...) > { >if (length(dim(x)) != 2) >stop("'x' must be a matrix-like object") >... > ) > > that is, just a regular function that checks that 'x' is matrix-like > based on its number of dimensions. If you really want to restrict to > matrix and DelayedMatrix only, replace the test with > >if (!(is.matrix(x) || is(x, "DelayedMatrix"))) >stop("'x' must be a matrix or DelayedMatrix object") > > The difference being that now the function will reject matrix-like > objects that are not matrix or DelayedMatrix objects (e.g. a Matrix > derivative from the Matrix package). > > Cheers, > H. > > > On 04/30/2018 09:29 AM, Stephanie M. Gogarten wrote: >> Rather than a class union, how about an internal function that is called by >> the methods for both matrix and DelayedArray: >> setGeneric("myNewRowMeans", function(x,...) { >> standardGeneric("myNewRowMeans")}) >> #' @importFrom DelayedArray rowMeans >> .myNewRowMeans <- function(x,...){ >> # a lot of code independent of x >> print("This is a lot of code shared regardless of class of x\n") >> # a lot of code that depends on x, but is dispatched by the functions >> called >> out<-rowMeans(x) >> #a lot of code based on output of out >> out<-out+1 >> return(out) >> } >> setMethod("myNewRowMeans", >> signature = "matrix", >> definition = function(x,...){ >> .myNewRowMeans(x,...) >> } >> ) >> setMethod("myNewRowMeans", >> signature = "DelayedArray", >> definition = function(x,...){ >> .myNewRowMeans(x,...) >> } >> ) >> On 4/30/18 9:10 AM, Tim Triche, Jr. wrote: >>> But if you merge methods like that, the error method can be that much more >>> difficult to identify. It took a couple of weeks to chase that bug down >>> properly, and it ended up down to rowMeans2 vs rowMeans. >>> >>> I suppose the merged/abstracted method allows to centralize any such >>> dispatch into one place and swap out ill-behaved methods once identified, >>> so as long as DelayedArray/DelayedMatrixStats quirks are >>> documented/understood, maybe it is better to create this union class? >>> >>> The Matrix/matrixStats/DelayedMatrix/DelayedMatrixStats situation has been >>> "interesting" in practical terms, as seemingly simple abstractions appear >>> to require more thought. That was my only point. >>> >>> >>> --t >>> >>> On Mon, Apr 30, 2018 at 11:28 AM, Martin Morgan < >>> martin.mor...@roswellpark.org> wrote: >>> But that issue will be fixed, so Tim's advice is inappropriate. On 04/30/2018 10:42 AM, Tim Triche, Jr. wrote: > Don't do that. Seriously, just don't. > > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Bioconductor_DelayedArray_issues_16&d=DwIDaQ&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=Rhy4i6H9xaY8HzWv9v_jhOnp5OyEpJcG52RP3nHorU8&s=olbErqY3_l7i45-WeTkaUNGalrQQr-7i59rhJVF6OGQ&e= > > > --t > > On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < > epur...@stat.berkeley.edu> wrote: > > Hello, >> >> I am trying to extend my package to handle `HDF5Matrix` class ( or more >> generally `DelayedArray`). I currently have S4 functions for `matrix` >> class. Usually I have a method for `SummarizedExperiment`, which will >> call >> call the method on `assay(x)` and I want the method to be able to deal >> with >> if `assay(x)` is a `DelayedArray`. >> >> Most of my functions, however, do not require separate code depending on >> whether `x` is a `matrix` or `DelayedArray`. They are making use of >> existing functions that will make that choice for me, e.g. rowMeans or >> subsetting. My goal right now is co
Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
Interesting. I tried something like that in the past i.e. start with a unary setClassUnion() but then got into problems when I tried to add new members to the union by **extending** the union class: https://stat.ethz.ch/pipermail/r-devel/2016-March/072489.html So it seems like I should have used setIs() instead. Then later I realized that I could just do with having the 'seed' slot of the DelayedArray class be "ANY" and having the validity method checking that the slot actually contains something for which dim() is not null. I actually adopted this "array-like == dim() is not NULL" approach everywhere in the package. It's simple and works well. H. On 04/30/2018 01:33 PM, Michael Lawrence wrote: It would be great to be able to define a matrix-like abstraction independent of 'matrix' and 'DelayedMatrix'. It could also encompass objects from the Matrix package and potentially other things. So you could define a parent class of 'matrix' using setClassUnion() and then use setIs() to establish further derivations: setClassUnion("MatrixLike", "matrix") setIs("DelayedMatrix", "MatrixLike") Michael On Mon, Apr 30, 2018 at 11:35 AM, Hervé Pagès wrote: The class union should probably be: setClassUnion("matrixOrDelayed", c("matrix", "DelayedMatrix")) i.e. use DelayedMatrix instead of DelayedArray. So in addition to the class union and to Stephanie's solution, which IMO are both valid solutions, you could also go for something like this: myNewRowMeans <- function(x,...) { if (length(dim(x)) != 2) stop("'x' must be a matrix-like object") ... ) that is, just a regular function that checks that 'x' is matrix-like based on its number of dimensions. If you really want to restrict to matrix and DelayedMatrix only, replace the test with if (!(is.matrix(x) || is(x, "DelayedMatrix"))) stop("'x' must be a matrix or DelayedMatrix object") The difference being that now the function will reject matrix-like objects that are not matrix or DelayedMatrix objects (e.g. a Matrix derivative from the Matrix package). Cheers, H. On 04/30/2018 09:29 AM, Stephanie M. Gogarten wrote: Rather than a class union, how about an internal function that is called by the methods for both matrix and DelayedArray: setGeneric("myNewRowMeans", function(x,...) { standardGeneric("myNewRowMeans")}) #' @importFrom DelayedArray rowMeans .myNewRowMeans <- function(x,...){ # a lot of code independent of x print("This is a lot of code shared regardless of class of x\n") # a lot of code that depends on x, but is dispatched by the functions called out<-rowMeans(x) #a lot of code based on output of out out<-out+1 return(out) } setMethod("myNewRowMeans", signature = "matrix", definition = function(x,...){ .myNewRowMeans(x,...) } ) setMethod("myNewRowMeans", signature = "DelayedArray", definition = function(x,...){ .myNewRowMeans(x,...) } ) On 4/30/18 9:10 AM, Tim Triche, Jr. wrote: But if you merge methods like that, the error method can be that much more difficult to identify. It took a couple of weeks to chase that bug down properly, and it ended up down to rowMeans2 vs rowMeans. I suppose the merged/abstracted method allows to centralize any such dispatch into one place and swap out ill-behaved methods once identified, so as long as DelayedArray/DelayedMatrixStats quirks are documented/understood, maybe it is better to create this union class? The Matrix/matrixStats/DelayedMatrix/DelayedMatrixStats situation has been "interesting" in practical terms, as seemingly simple abstractions appear to require more thought. That was my only point. --t On Mon, Apr 30, 2018 at 11:28 AM, Martin Morgan < martin.mor...@roswellpark.org> wrote: But that issue will be fixed, so Tim's advice is inappropriate. On 04/30/2018 10:42 AM, Tim Triche, Jr. wrote: Don't do that. Seriously, just don't. https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Bioconductor_DelayedArray_issues_16&d=DwIDaQ&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=Rhy4i6H9xaY8HzWv9v_jhOnp5OyEpJcG52RP3nHorU8&s=olbErqY3_l7i45-WeTkaUNGalrQQr-7i59rhJVF6OGQ&e= --t On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < epur...@stat.berkeley.edu> wrote: Hello, I am trying to extend my package to handle `HDF5Matrix` class ( or more generally `DelayedArray`). I currently have S4 functions for `matrix` class. Usually I have a method for `SummarizedExperiment`, which will call call the method on `assay(x)` and I want the method to be able to deal with if `assay(x)` is a `DelayedArray`. Most of my functions, however, do not require separate code depending on whether `x` is a `matrix` or `DelayedArray`. They are making use of existing functions that will make that choice for me, e.g. rowMeans or subsetting. My goal right now is compatibility, not cleverness
Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
It would be great to be able to define a matrix-like abstraction independent of 'matrix' and 'DelayedMatrix'. It could also encompass objects from the Matrix package and potentially other things. So you could define a parent class of 'matrix' using setClassUnion() and then use setIs() to establish further derivations: setClassUnion("MatrixLike", "matrix") setIs("DelayedMatrix", "MatrixLike") Michael On Mon, Apr 30, 2018 at 11:35 AM, Hervé Pagès wrote: > The class union should probably be: > > setClassUnion("matrixOrDelayed", c("matrix", "DelayedMatrix")) > > i.e. use DelayedMatrix instead of DelayedArray. > > So in addition to the class union and to Stephanie's solution, which > IMO are both valid solutions, you could also go for something like this: > > myNewRowMeans <- function(x,...) > { > if (length(dim(x)) != 2) > stop("'x' must be a matrix-like object") > ... > ) > > that is, just a regular function that checks that 'x' is matrix-like > based on its number of dimensions. If you really want to restrict to > matrix and DelayedMatrix only, replace the test with > > if (!(is.matrix(x) || is(x, "DelayedMatrix"))) > stop("'x' must be a matrix or DelayedMatrix object") > > The difference being that now the function will reject matrix-like > objects that are not matrix or DelayedMatrix objects (e.g. a Matrix > derivative from the Matrix package). > > Cheers, > H. > > > > On 04/30/2018 09:29 AM, Stephanie M. Gogarten wrote: >> >> Rather than a class union, how about an internal function that is called >> by the methods for both matrix and DelayedArray: >> >> >> setGeneric("myNewRowMeans", function(x,...) { >> standardGeneric("myNewRowMeans")}) >> >> #' @importFrom DelayedArray rowMeans >> .myNewRowMeans <- function(x,...){ >> # a lot of code independent of x >> print("This is a lot of code shared regardless of class of x\n") >> # a lot of code that depends on x, but is dispatched by the functions >> called >> out<-rowMeans(x) >> #a lot of code based on output of out >> out<-out+1 >> return(out) >> } >> >> setMethod("myNewRowMeans", >>signature = "matrix", >>definition = function(x,...){ >>.myNewRowMeans(x,...) >>} >> ) >> >> setMethod("myNewRowMeans", >>signature = "DelayedArray", >>definition = function(x,...){ >>.myNewRowMeans(x,...) >>} >> ) >> >> >> On 4/30/18 9:10 AM, Tim Triche, Jr. wrote: >>> >>> But if you merge methods like that, the error method can be that much >>> more >>> difficult to identify. It took a couple of weeks to chase that bug down >>> properly, and it ended up down to rowMeans2 vs rowMeans. >>> >>> I suppose the merged/abstracted method allows to centralize any such >>> dispatch into one place and swap out ill-behaved methods once identified, >>> so as long as DelayedArray/DelayedMatrixStats quirks are >>> documented/understood, maybe it is better to create this union class? >>> >>> The Matrix/matrixStats/DelayedMatrix/DelayedMatrixStats situation has >>> been >>> "interesting" in practical terms, as seemingly simple abstractions appear >>> to require more thought. That was my only point. >>> >>> >>> --t >>> >>> On Mon, Apr 30, 2018 at 11:28 AM, Martin Morgan < >>> martin.mor...@roswellpark.org> wrote: >>> But that issue will be fixed, so Tim's advice is inappropriate. On 04/30/2018 10:42 AM, Tim Triche, Jr. wrote: > Don't do that. Seriously, just don't. > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Bioconductor_DelayedArray_issues_16&d=DwIDaQ&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=Rhy4i6H9xaY8HzWv9v_jhOnp5OyEpJcG52RP3nHorU8&s=olbErqY3_l7i45-WeTkaUNGalrQQr-7i59rhJVF6OGQ&e= > > --t > > On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < > epur...@stat.berkeley.edu> wrote: > > Hello, >> >> >> I am trying to extend my package to handle `HDF5Matrix` class ( or >> more >> generally `DelayedArray`). I currently have S4 functions for `matrix` >> class. Usually I have a method for `SummarizedExperiment`, which will >> call >> call the method on `assay(x)` and I want the method to be able to deal >> with >> if `assay(x)` is a `DelayedArray`. >> >> Most of my functions, however, do not require separate code depending >> on >> whether `x` is a `matrix` or `DelayedArray`. They are making use of >> existing functions that will make that choice for me, e.g. rowMeans or >> subsetting. My goal right now is compatibility, not cleverness, and >> I'm >> not >> creating HDF5 methods to handle other cases. (If something doesn't >> currently exist, then I just enclose `x` with `data.matrix` or >> `as.matrix` >> and call the matrix into memory — for cleanliness and ease in updating >> with >> appropriate methods in f
Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
The class union should probably be: setClassUnion("matrixOrDelayed", c("matrix", "DelayedMatrix")) i.e. use DelayedMatrix instead of DelayedArray. So in addition to the class union and to Stephanie's solution, which IMO are both valid solutions, you could also go for something like this: myNewRowMeans <- function(x,...) { if (length(dim(x)) != 2) stop("'x' must be a matrix-like object") ... ) that is, just a regular function that checks that 'x' is matrix-like based on its number of dimensions. If you really want to restrict to matrix and DelayedMatrix only, replace the test with if (!(is.matrix(x) || is(x, "DelayedMatrix"))) stop("'x' must be a matrix or DelayedMatrix object") The difference being that now the function will reject matrix-like objects that are not matrix or DelayedMatrix objects (e.g. a Matrix derivative from the Matrix package). Cheers, H. On 04/30/2018 09:29 AM, Stephanie M. Gogarten wrote: Rather than a class union, how about an internal function that is called by the methods for both matrix and DelayedArray: setGeneric("myNewRowMeans", function(x,...) { standardGeneric("myNewRowMeans")}) #' @importFrom DelayedArray rowMeans .myNewRowMeans <- function(x,...){ # a lot of code independent of x print("This is a lot of code shared regardless of class of x\n") # a lot of code that depends on x, but is dispatched by the functions called out<-rowMeans(x) #a lot of code based on output of out out<-out+1 return(out) } setMethod("myNewRowMeans", signature = "matrix", definition = function(x,...){ .myNewRowMeans(x,...) } ) setMethod("myNewRowMeans", signature = "DelayedArray", definition = function(x,...){ .myNewRowMeans(x,...) } ) On 4/30/18 9:10 AM, Tim Triche, Jr. wrote: But if you merge methods like that, the error method can be that much more difficult to identify. It took a couple of weeks to chase that bug down properly, and it ended up down to rowMeans2 vs rowMeans. I suppose the merged/abstracted method allows to centralize any such dispatch into one place and swap out ill-behaved methods once identified, so as long as DelayedArray/DelayedMatrixStats quirks are documented/understood, maybe it is better to create this union class? The Matrix/matrixStats/DelayedMatrix/DelayedMatrixStats situation has been "interesting" in practical terms, as seemingly simple abstractions appear to require more thought. That was my only point. --t On Mon, Apr 30, 2018 at 11:28 AM, Martin Morgan < martin.mor...@roswellpark.org> wrote: But that issue will be fixed, so Tim's advice is inappropriate. On 04/30/2018 10:42 AM, Tim Triche, Jr. wrote: Don't do that. Seriously, just don't. https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Bioconductor_DelayedArray_issues_16&d=DwIDaQ&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=Rhy4i6H9xaY8HzWv9v_jhOnp5OyEpJcG52RP3nHorU8&s=olbErqY3_l7i45-WeTkaUNGalrQQr-7i59rhJVF6OGQ&e= --t On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < epur...@stat.berkeley.edu> wrote: Hello, I am trying to extend my package to handle `HDF5Matrix` class ( or more generally `DelayedArray`). I currently have S4 functions for `matrix` class. Usually I have a method for `SummarizedExperiment`, which will call call the method on `assay(x)` and I want the method to be able to deal with if `assay(x)` is a `DelayedArray`. Most of my functions, however, do not require separate code depending on whether `x` is a `matrix` or `DelayedArray`. They are making use of existing functions that will make that choice for me, e.g. rowMeans or subsetting. My goal right now is compatibility, not cleverness, and I'm not creating HDF5 methods to handle other cases. (If something doesn't currently exist, then I just enclose `x` with `data.matrix` or `as.matrix` and call the matrix into memory — for cleanliness and ease in updating with appropriate methods in future, I could make separate S4 functions for these specific tasks to dispatch, but that's outside of the scope of my question). So for simplicity assume I don't really need to dispatch *my code* -- that the methods I'm going to use do that. The natural solution for me seem to use `setClassUnion` and I was wondering if such a virtual class already exists? Or is there a better way to handle this? Here's a simple example, using `rowMeans` as my example: ``` setGeneric("myNewRowMeans", function(x,...) { standardGeneric(" myNewRowMeans")}) setClassUnion("matrixOrDelayed",members=c("matrix", "DelayedArray")) #' @importFrom DelayedArray rowMeans setMethod("myNewRowMeans", signature = "matrixOrDelayed", definition = function(x,...){ # a lot of code independent of x print("This is a lot of code shared regardless of class of x\n")
Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
Just to mention that the issue with rowSums() on a big DelayedMatrix objects that you are referring to Tim is duly noted and will be one of my first priorities once we're done with the release process. Cheers, H. On 04/30/2018 09:57 AM, Tim Triche, Jr. wrote: much obliged -- and the packages are terrific, I am not surprised that a big step is accompanied with some growing pains. Thanks to you and Herve and Keegan for enthusiastically chasing down, and spending your time fixing, this and other bugs. Having fiddled with bigMemory and bigMatrix backends for SummarizedExperiment over the years, I know how many kicks and stings exist under the covers, and greatly appreciate your (plural) efforts. --t On Mon, Apr 30, 2018 at 12:30 PM, Peter Hickey wrote: Tim: As the developer of DelayedMatrixStats (and enthusiastic 'canary down the coal mine' user-dev of DelayedArray) I'm obviously invested in reducing the confusion around these packages I'm going to write some blog posts-cum-vignettes-cum-F1000 around these issues over the coming weeks, with the ultimate goal of improving the packages themselves. Pete On Mon., 30 Apr. 2018, 12:11 pm Tim Triche, Jr., wrote: But if you merge methods like that, the error method can be that much more difficult to identify. It took a couple of weeks to chase that bug down properly, and it ended up down to rowMeans2 vs rowMeans. I suppose the merged/abstracted method allows to centralize any such dispatch into one place and swap out ill-behaved methods once identified, so as long as DelayedArray/DelayedMatrixStats quirks are documented/understood, maybe it is better to create this union class? The Matrix/matrixStats/DelayedMatrix/DelayedMatrixStats situation has been "interesting" in practical terms, as seemingly simple abstractions appear to require more thought. That was my only point. --t On Mon, Apr 30, 2018 at 11:28 AM, Martin Morgan < martin.mor...@roswellpark.org> wrote: But that issue will be fixed, so Tim's advice is inappropriate. On 04/30/2018 10:42 AM, Tim Triche, Jr. wrote: Don't do that. Seriously, just don't. https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Bioconductor_DelayedArray_issues_16&d=DwIFaQ&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=WCuDvGWmDrT5ZoYylftzjbrlaEu-lOxIIJaNJgn6itQ&s=kC-1EPn1Q-IuXzwk3-l-pJNp8zu4tXBnAP-1VPWKlbs&e= --t On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < epur...@stat.berkeley.edu> wrote: Hello, I am trying to extend my package to handle `HDF5Matrix` class ( or more generally `DelayedArray`). I currently have S4 functions for `matrix` class. Usually I have a method for `SummarizedExperiment`, which will call call the method on `assay(x)` and I want the method to be able to deal with if `assay(x)` is a `DelayedArray`. Most of my functions, however, do not require separate code depending on whether `x` is a `matrix` or `DelayedArray`. They are making use of existing functions that will make that choice for me, e.g. rowMeans or subsetting. My goal right now is compatibility, not cleverness, and I'm not creating HDF5 methods to handle other cases. (If something doesn't currently exist, then I just enclose `x` with `data.matrix` or `as.matrix` and call the matrix into memory — for cleanliness and ease in updating with appropriate methods in future, I could make separate S4 functions for these specific tasks to dispatch, but that's outside of the scope of my question). So for simplicity assume I don't really need to dispatch *my code* -- that the methods I'm going to use do that. The natural solution for me seem to use `setClassUnion` and I was wondering if such a virtual class already exists? Or is there a better way to handle this? Here's a simple example, using `rowMeans` as my example: ``` setGeneric("myNewRowMeans", function(x,...) { standardGeneric(" myNewRowMeans")}) setClassUnion("matrixOrDelayed",members=c("matrix", "DelayedArray")) #' @importFrom DelayedArray rowMeans setMethod("myNewRowMeans", signature = "matrixOrDelayed", definition = function(x,...){ # a lot of code independent of x print("This is a lot of code shared regardless of class of x\n") # a lot of code that depends on x, but is dispatched by the functions called out<-rowMeans(x) #a lot of code based on output of out out<-out+1 return(out) } ) ``` ___ Bioc-devel@r-project.org mailing list https://urldefense.proofpoint.com/v2/url?u=https-3A__stat.ethz.ch_mailman_listinfo_bioc-2Ddevel&d=DwIFaQ&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=WCuDvGWmDrT5ZoYylftzjbrlaEu-lOxIIJaNJgn6itQ&s=_3ZIrKjXNYWYMKKDBvbn1aNtGMB6rfqfhs-zU_P5_ug&e= [[alternative HTML version deleted]] __
Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
much obliged -- and the packages are terrific, I am not surprised that a big step is accompanied with some growing pains. Thanks to you and Herve and Keegan for enthusiastically chasing down, and spending your time fixing, this and other bugs. Having fiddled with bigMemory and bigMatrix backends for SummarizedExperiment over the years, I know how many kicks and stings exist under the covers, and greatly appreciate your (plural) efforts. --t On Mon, Apr 30, 2018 at 12:30 PM, Peter Hickey wrote: > Tim: As the developer of DelayedMatrixStats (and enthusiastic 'canary down > the coal mine' user-dev of DelayedArray) I'm obviously invested in reducing > the confusion around these packages > > I'm going to write some blog posts-cum-vignettes-cum-F1000 around these > issues over the coming weeks, with the ultimate goal of improving the > packages themselves. > > Pete > > > > On Mon., 30 Apr. 2018, 12:11 pm Tim Triche, Jr., > wrote: > >> But if you merge methods like that, the error method can be that much more >> difficult to identify. It took a couple of weeks to chase that bug down >> properly, and it ended up down to rowMeans2 vs rowMeans. >> >> I suppose the merged/abstracted method allows to centralize any such >> dispatch into one place and swap out ill-behaved methods once identified, >> so as long as DelayedArray/DelayedMatrixStats quirks are >> documented/understood, maybe it is better to create this union class? >> >> The Matrix/matrixStats/DelayedMatrix/DelayedMatrixStats situation has >> been >> "interesting" in practical terms, as seemingly simple abstractions appear >> to require more thought. That was my only point. >> >> >> --t >> >> On Mon, Apr 30, 2018 at 11:28 AM, Martin Morgan < >> martin.mor...@roswellpark.org> wrote: >> >> > But that issue will be fixed, so Tim's advice is inappropriate. >> > >> > >> > On 04/30/2018 10:42 AM, Tim Triche, Jr. wrote: >> > >> >> Don't do that. Seriously, just don't. >> >> >> >> https://github.com/Bioconductor/DelayedArray/issues/16 >> >> >> >> --t >> >> >> >> On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < >> >> epur...@stat.berkeley.edu> wrote: >> >> >> >> Hello, >> >>> >> >>> I am trying to extend my package to handle `HDF5Matrix` class ( or >> more >> >>> generally `DelayedArray`). I currently have S4 functions for `matrix` >> >>> class. Usually I have a method for `SummarizedExperiment`, which will >> >>> call >> >>> call the method on `assay(x)` and I want the method to be able to deal >> >>> with >> >>> if `assay(x)` is a `DelayedArray`. >> >>> >> >>> Most of my functions, however, do not require separate code depending >> on >> >>> whether `x` is a `matrix` or `DelayedArray`. They are making use of >> >>> existing functions that will make that choice for me, e.g. rowMeans or >> >>> subsetting. My goal right now is compatibility, not cleverness, and >> I'm >> >>> not >> >>> creating HDF5 methods to handle other cases. (If something doesn't >> >>> currently exist, then I just enclose `x` with `data.matrix` or >> >>> `as.matrix` >> >>> and call the matrix into memory — for cleanliness and ease in updating >> >>> with >> >>> appropriate methods in future, I could make separate S4 functions for >> >>> these >> >>> specific tasks to dispatch, but that's outside of the scope of my >> >>> question). So for simplicity assume I don't really need to dispatch >> *my >> >>> code* -- that the methods I'm going to use do that. >> >>> >> >>> The natural solution for me seem to use `setClassUnion` and I was >> >>> wondering if such a virtual class already exists? Or is there a better >> >>> way >> >>> to handle this? >> >>> >> >>> Here's a simple example, using `rowMeans` as my example: >> >>> >> >>> ``` >> >>> setGeneric("myNewRowMeans", function(x,...) { standardGeneric(" >> >>> myNewRowMeans")}) >> >>> setClassUnion("matrixOrDelayed",members=c("matrix", "DelayedArray")) >> >>> >> >>> #' @importFrom DelayedArray rowMeans >> >>> setMethod("myNewRowMeans", >> >>>signature = "matrixOrDelayed", >> >>>definition = function(x,...){ >> >>> # a lot of code independent of x >> >>> print("This is a lot of code shared >> regardless >> >>> of >> >>> class of x\n") >> >>> # a lot of code that depends on x, but is >> >>> dispatched by the functions called >> >>> out<-rowMeans(x) >> >>> #a lot of code based on output of out >> >>> out<-out+1 >> >>> return(out) >> >>> } >> >>> ) >> >>> ``` >> >>> >> >>> ___ >> >>> Bioc-devel@r-project.org mailing list >> >>> https://stat.ethz.ch/mailman/listinfo/bioc-devel >> >>> >> >>> >> >> [[alternative HTML version deleted]] >> >> >> >> ___ >> >> Bioc-devel@r-project.org mailing list >> >> https://stat.ethz.ch/mailman/listinfo/bioc
Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
Rather than a class union, how about an internal function that is called by the methods for both matrix and DelayedArray: setGeneric("myNewRowMeans", function(x,...) { standardGeneric("myNewRowMeans")}) #' @importFrom DelayedArray rowMeans .myNewRowMeans <- function(x,...){ # a lot of code independent of x print("This is a lot of code shared regardless of class of x\n") # a lot of code that depends on x, but is dispatched by the functions called out<-rowMeans(x) #a lot of code based on output of out out<-out+1 return(out) } setMethod("myNewRowMeans", signature = "matrix", definition = function(x,...){ .myNewRowMeans(x,...) } ) setMethod("myNewRowMeans", signature = "DelayedArray", definition = function(x,...){ .myNewRowMeans(x,...) } ) On 4/30/18 9:10 AM, Tim Triche, Jr. wrote: But if you merge methods like that, the error method can be that much more difficult to identify. It took a couple of weeks to chase that bug down properly, and it ended up down to rowMeans2 vs rowMeans. I suppose the merged/abstracted method allows to centralize any such dispatch into one place and swap out ill-behaved methods once identified, so as long as DelayedArray/DelayedMatrixStats quirks are documented/understood, maybe it is better to create this union class? The Matrix/matrixStats/DelayedMatrix/DelayedMatrixStats situation has been "interesting" in practical terms, as seemingly simple abstractions appear to require more thought. That was my only point. --t On Mon, Apr 30, 2018 at 11:28 AM, Martin Morgan < martin.mor...@roswellpark.org> wrote: But that issue will be fixed, so Tim's advice is inappropriate. On 04/30/2018 10:42 AM, Tim Triche, Jr. wrote: Don't do that. Seriously, just don't. https://github.com/Bioconductor/DelayedArray/issues/16 --t On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < epur...@stat.berkeley.edu> wrote: Hello, I am trying to extend my package to handle `HDF5Matrix` class ( or more generally `DelayedArray`). I currently have S4 functions for `matrix` class. Usually I have a method for `SummarizedExperiment`, which will call call the method on `assay(x)` and I want the method to be able to deal with if `assay(x)` is a `DelayedArray`. Most of my functions, however, do not require separate code depending on whether `x` is a `matrix` or `DelayedArray`. They are making use of existing functions that will make that choice for me, e.g. rowMeans or subsetting. My goal right now is compatibility, not cleverness, and I'm not creating HDF5 methods to handle other cases. (If something doesn't currently exist, then I just enclose `x` with `data.matrix` or `as.matrix` and call the matrix into memory — for cleanliness and ease in updating with appropriate methods in future, I could make separate S4 functions for these specific tasks to dispatch, but that's outside of the scope of my question). So for simplicity assume I don't really need to dispatch *my code* -- that the methods I'm going to use do that. The natural solution for me seem to use `setClassUnion` and I was wondering if such a virtual class already exists? Or is there a better way to handle this? Here's a simple example, using `rowMeans` as my example: ``` setGeneric("myNewRowMeans", function(x,...) { standardGeneric(" myNewRowMeans")}) setClassUnion("matrixOrDelayed",members=c("matrix", "DelayedArray")) #' @importFrom DelayedArray rowMeans setMethod("myNewRowMeans", signature = "matrixOrDelayed", definition = function(x,...){ # a lot of code independent of x print("This is a lot of code shared regardless of class of x\n") # a lot of code that depends on x, but is dispatched by the functions called out<-rowMeans(x) #a lot of code based on output of out out<-out+1 return(out) } ) ``` ___ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel [[alternative HTML version deleted]] ___ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel This email message may contain legally privileged and/or confidential information. If you are not the intended recipient(s), or the employee or agent responsible for the delivery of this message to the intended recipient(s), you are hereby notified that any disclosure, copying, distribution, or use of this email message is prohibited. If you have received this message in error, please notify the sender immediately by e-mail and delete this email message from your computer. Thank you. [[alternative HTML version deleted]] ___
Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
Tim: As the developer of DelayedMatrixStats (and enthusiastic 'canary down the coal mine' user-dev of DelayedArray) I'm obviously invested in reducing the confusion around these packages I'm going to write some blog posts-cum-vignettes-cum-F1000 around these issues over the coming weeks, with the ultimate goal of improving the packages themselves. Pete On Mon., 30 Apr. 2018, 12:11 pm Tim Triche, Jr., wrote: > But if you merge methods like that, the error method can be that much more > difficult to identify. It took a couple of weeks to chase that bug down > properly, and it ended up down to rowMeans2 vs rowMeans. > > I suppose the merged/abstracted method allows to centralize any such > dispatch into one place and swap out ill-behaved methods once identified, > so as long as DelayedArray/DelayedMatrixStats quirks are > documented/understood, maybe it is better to create this union class? > > The Matrix/matrixStats/DelayedMatrix/DelayedMatrixStats situation has been > "interesting" in practical terms, as seemingly simple abstractions appear > to require more thought. That was my only point. > > > --t > > On Mon, Apr 30, 2018 at 11:28 AM, Martin Morgan < > martin.mor...@roswellpark.org> wrote: > > > But that issue will be fixed, so Tim's advice is inappropriate. > > > > > > On 04/30/2018 10:42 AM, Tim Triche, Jr. wrote: > > > >> Don't do that. Seriously, just don't. > >> > >> https://github.com/Bioconductor/DelayedArray/issues/16 > >> > >> --t > >> > >> On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < > >> epur...@stat.berkeley.edu> wrote: > >> > >> Hello, > >>> > >>> I am trying to extend my package to handle `HDF5Matrix` class ( or more > >>> generally `DelayedArray`). I currently have S4 functions for `matrix` > >>> class. Usually I have a method for `SummarizedExperiment`, which will > >>> call > >>> call the method on `assay(x)` and I want the method to be able to deal > >>> with > >>> if `assay(x)` is a `DelayedArray`. > >>> > >>> Most of my functions, however, do not require separate code depending > on > >>> whether `x` is a `matrix` or `DelayedArray`. They are making use of > >>> existing functions that will make that choice for me, e.g. rowMeans or > >>> subsetting. My goal right now is compatibility, not cleverness, and I'm > >>> not > >>> creating HDF5 methods to handle other cases. (If something doesn't > >>> currently exist, then I just enclose `x` with `data.matrix` or > >>> `as.matrix` > >>> and call the matrix into memory — for cleanliness and ease in updating > >>> with > >>> appropriate methods in future, I could make separate S4 functions for > >>> these > >>> specific tasks to dispatch, but that's outside of the scope of my > >>> question). So for simplicity assume I don't really need to dispatch *my > >>> code* -- that the methods I'm going to use do that. > >>> > >>> The natural solution for me seem to use `setClassUnion` and I was > >>> wondering if such a virtual class already exists? Or is there a better > >>> way > >>> to handle this? > >>> > >>> Here's a simple example, using `rowMeans` as my example: > >>> > >>> ``` > >>> setGeneric("myNewRowMeans", function(x,...) { standardGeneric(" > >>> myNewRowMeans")}) > >>> setClassUnion("matrixOrDelayed",members=c("matrix", "DelayedArray")) > >>> > >>> #' @importFrom DelayedArray rowMeans > >>> setMethod("myNewRowMeans", > >>>signature = "matrixOrDelayed", > >>>definition = function(x,...){ > >>> # a lot of code independent of x > >>> print("This is a lot of code shared regardless > >>> of > >>> class of x\n") > >>> # a lot of code that depends on x, but is > >>> dispatched by the functions called > >>> out<-rowMeans(x) > >>> #a lot of code based on output of out > >>> out<-out+1 > >>> return(out) > >>> } > >>> ) > >>> ``` > >>> > >>> ___ > >>> Bioc-devel@r-project.org mailing list > >>> https://stat.ethz.ch/mailman/listinfo/bioc-devel > >>> > >>> > >> [[alternative HTML version deleted]] > >> > >> ___ > >> Bioc-devel@r-project.org mailing list > >> https://stat.ethz.ch/mailman/listinfo/bioc-devel > >> > >> > > > > This email message may contain legally privileged and/or confidential > > information. If you are not the intended recipient(s), or the employee > or > > agent responsible for the delivery of this message to the intended > > recipient(s), you are hereby notified that any disclosure, copying, > > distribution, or use of this email message is prohibited. If you have > > received this message in error, please notify the sender immediately by > > e-mail and delete this email message from your computer. Thank you. > > > > [[alternative HTML version deleted]] > >
Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
But if you merge methods like that, the error method can be that much more difficult to identify. It took a couple of weeks to chase that bug down properly, and it ended up down to rowMeans2 vs rowMeans. I suppose the merged/abstracted method allows to centralize any such dispatch into one place and swap out ill-behaved methods once identified, so as long as DelayedArray/DelayedMatrixStats quirks are documented/understood, maybe it is better to create this union class? The Matrix/matrixStats/DelayedMatrix/DelayedMatrixStats situation has been "interesting" in practical terms, as seemingly simple abstractions appear to require more thought. That was my only point. --t On Mon, Apr 30, 2018 at 11:28 AM, Martin Morgan < martin.mor...@roswellpark.org> wrote: > But that issue will be fixed, so Tim's advice is inappropriate. > > > On 04/30/2018 10:42 AM, Tim Triche, Jr. wrote: > >> Don't do that. Seriously, just don't. >> >> https://github.com/Bioconductor/DelayedArray/issues/16 >> >> --t >> >> On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < >> epur...@stat.berkeley.edu> wrote: >> >> Hello, >>> >>> I am trying to extend my package to handle `HDF5Matrix` class ( or more >>> generally `DelayedArray`). I currently have S4 functions for `matrix` >>> class. Usually I have a method for `SummarizedExperiment`, which will >>> call >>> call the method on `assay(x)` and I want the method to be able to deal >>> with >>> if `assay(x)` is a `DelayedArray`. >>> >>> Most of my functions, however, do not require separate code depending on >>> whether `x` is a `matrix` or `DelayedArray`. They are making use of >>> existing functions that will make that choice for me, e.g. rowMeans or >>> subsetting. My goal right now is compatibility, not cleverness, and I'm >>> not >>> creating HDF5 methods to handle other cases. (If something doesn't >>> currently exist, then I just enclose `x` with `data.matrix` or >>> `as.matrix` >>> and call the matrix into memory — for cleanliness and ease in updating >>> with >>> appropriate methods in future, I could make separate S4 functions for >>> these >>> specific tasks to dispatch, but that's outside of the scope of my >>> question). So for simplicity assume I don't really need to dispatch *my >>> code* -- that the methods I'm going to use do that. >>> >>> The natural solution for me seem to use `setClassUnion` and I was >>> wondering if such a virtual class already exists? Or is there a better >>> way >>> to handle this? >>> >>> Here's a simple example, using `rowMeans` as my example: >>> >>> ``` >>> setGeneric("myNewRowMeans", function(x,...) { standardGeneric(" >>> myNewRowMeans")}) >>> setClassUnion("matrixOrDelayed",members=c("matrix", "DelayedArray")) >>> >>> #' @importFrom DelayedArray rowMeans >>> setMethod("myNewRowMeans", >>>signature = "matrixOrDelayed", >>>definition = function(x,...){ >>> # a lot of code independent of x >>> print("This is a lot of code shared regardless >>> of >>> class of x\n") >>> # a lot of code that depends on x, but is >>> dispatched by the functions called >>> out<-rowMeans(x) >>> #a lot of code based on output of out >>> out<-out+1 >>> return(out) >>> } >>> ) >>> ``` >>> >>> ___ >>> Bioc-devel@r-project.org mailing list >>> https://stat.ethz.ch/mailman/listinfo/bioc-devel >>> >>> >> [[alternative HTML version deleted]] >> >> ___ >> Bioc-devel@r-project.org mailing list >> https://stat.ethz.ch/mailman/listinfo/bioc-devel >> >> > > This email message may contain legally privileged and/or confidential > information. If you are not the intended recipient(s), or the employee or > agent responsible for the delivery of this message to the intended > recipient(s), you are hereby notified that any disclosure, copying, > distribution, or use of this email message is prohibited. If you have > received this message in error, please notify the sender immediately by > e-mail and delete this email message from your computer. Thank you. > [[alternative HTML version deleted]] ___ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel
Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
But that issue will be fixed, so Tim's advice is inappropriate. On 04/30/2018 10:42 AM, Tim Triche, Jr. wrote: Don't do that. Seriously, just don't. https://github.com/Bioconductor/DelayedArray/issues/16 --t On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < epur...@stat.berkeley.edu> wrote: Hello, I am trying to extend my package to handle `HDF5Matrix` class ( or more generally `DelayedArray`). I currently have S4 functions for `matrix` class. Usually I have a method for `SummarizedExperiment`, which will call call the method on `assay(x)` and I want the method to be able to deal with if `assay(x)` is a `DelayedArray`. Most of my functions, however, do not require separate code depending on whether `x` is a `matrix` or `DelayedArray`. They are making use of existing functions that will make that choice for me, e.g. rowMeans or subsetting. My goal right now is compatibility, not cleverness, and I'm not creating HDF5 methods to handle other cases. (If something doesn't currently exist, then I just enclose `x` with `data.matrix` or `as.matrix` and call the matrix into memory — for cleanliness and ease in updating with appropriate methods in future, I could make separate S4 functions for these specific tasks to dispatch, but that's outside of the scope of my question). So for simplicity assume I don't really need to dispatch *my code* -- that the methods I'm going to use do that. The natural solution for me seem to use `setClassUnion` and I was wondering if such a virtual class already exists? Or is there a better way to handle this? Here's a simple example, using `rowMeans` as my example: ``` setGeneric("myNewRowMeans", function(x,...) { standardGeneric(" myNewRowMeans")}) setClassUnion("matrixOrDelayed",members=c("matrix", "DelayedArray")) #' @importFrom DelayedArray rowMeans setMethod("myNewRowMeans", signature = "matrixOrDelayed", definition = function(x,...){ # a lot of code independent of x print("This is a lot of code shared regardless of class of x\n") # a lot of code that depends on x, but is dispatched by the functions called out<-rowMeans(x) #a lot of code based on output of out out<-out+1 return(out) } ) ``` ___ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel [[alternative HTML version deleted]] ___ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel This email message may contain legally privileged and/or...{{dropped:2}} ___ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel
Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
Don't do that. Seriously, just don't. https://github.com/Bioconductor/DelayedArray/issues/16 --t On Mon, Apr 30, 2018 at 10:02 AM, Elizabeth Purdom < epur...@stat.berkeley.edu> wrote: > Hello, > > I am trying to extend my package to handle `HDF5Matrix` class ( or more > generally `DelayedArray`). I currently have S4 functions for `matrix` > class. Usually I have a method for `SummarizedExperiment`, which will call > call the method on `assay(x)` and I want the method to be able to deal with > if `assay(x)` is a `DelayedArray`. > > Most of my functions, however, do not require separate code depending on > whether `x` is a `matrix` or `DelayedArray`. They are making use of > existing functions that will make that choice for me, e.g. rowMeans or > subsetting. My goal right now is compatibility, not cleverness, and I'm not > creating HDF5 methods to handle other cases. (If something doesn't > currently exist, then I just enclose `x` with `data.matrix` or `as.matrix` > and call the matrix into memory — for cleanliness and ease in updating with > appropriate methods in future, I could make separate S4 functions for these > specific tasks to dispatch, but that's outside of the scope of my > question). So for simplicity assume I don't really need to dispatch *my > code* -- that the methods I'm going to use do that. > > The natural solution for me seem to use `setClassUnion` and I was > wondering if such a virtual class already exists? Or is there a better way > to handle this? > > Here's a simple example, using `rowMeans` as my example: > > ``` > setGeneric("myNewRowMeans", function(x,...) { standardGeneric(" > myNewRowMeans")}) > setClassUnion("matrixOrDelayed",members=c("matrix", "DelayedArray")) > > #' @importFrom DelayedArray rowMeans > setMethod("myNewRowMeans", > signature = "matrixOrDelayed", > definition = function(x,...){ > # a lot of code independent of x > print("This is a lot of code shared regardless of > class of x\n") > # a lot of code that depends on x, but is > dispatched by the functions called > out<-rowMeans(x) > #a lot of code based on output of out > out<-out+1 > return(out) > } > ) > ``` > > ___ > Bioc-devel@r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/bioc-devel > [[alternative HTML version deleted]] ___ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel
[Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)
Hello, I am trying to extend my package to handle `HDF5Matrix` class ( or more generally `DelayedArray`). I currently have S4 functions for `matrix` class. Usually I have a method for `SummarizedExperiment`, which will call call the method on `assay(x)` and I want the method to be able to deal with if `assay(x)` is a `DelayedArray`. Most of my functions, however, do not require separate code depending on whether `x` is a `matrix` or `DelayedArray`. They are making use of existing functions that will make that choice for me, e.g. rowMeans or subsetting. My goal right now is compatibility, not cleverness, and I'm not creating HDF5 methods to handle other cases. (If something doesn't currently exist, then I just enclose `x` with `data.matrix` or `as.matrix` and call the matrix into memory — for cleanliness and ease in updating with appropriate methods in future, I could make separate S4 functions for these specific tasks to dispatch, but that's outside of the scope of my question). So for simplicity assume I don't really need to dispatch *my code* -- that the methods I'm going to use do that. The natural solution for me seem to use `setClassUnion` and I was wondering if such a virtual class already exists? Or is there a better way to handle this? Here's a simple example, using `rowMeans` as my example: ``` setGeneric("myNewRowMeans", function(x,...) { standardGeneric("myNewRowMeans")}) setClassUnion("matrixOrDelayed",members=c("matrix", "DelayedArray")) #' @importFrom DelayedArray rowMeans setMethod("myNewRowMeans", signature = "matrixOrDelayed", definition = function(x,...){ # a lot of code independent of x print("This is a lot of code shared regardless of class of x\n") # a lot of code that depends on x, but is dispatched by the functions called out<-rowMeans(x) #a lot of code based on output of out out<-out+1 return(out) } ) ``` ___ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel