Re: [Bioc-devel] Virtual class for `matrix` and `DelayedArray`? (or better strategy for dealing with them both)

2018-05-02 Thread Kasper Daniel Hansen
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)

2018-05-02 Thread Elizabeth Purdom
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)

2018-04-30 Thread Hervé Pagès

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)

2018-04-30 Thread Michael Lawrence
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)

2018-04-30 Thread Hervé Pagès

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)

2018-04-30 Thread Hervé Pagès

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)

2018-04-30 Thread Tim Triche, Jr.
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)

2018-04-30 Thread Stephanie M. Gogarten
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)

2018-04-30 Thread Peter Hickey
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)

2018-04-30 Thread Tim Triche, Jr.
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)

2018-04-30 Thread Martin Morgan

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)

2018-04-30 Thread Tim Triche, Jr.
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)

2018-04-30 Thread Elizabeth Purdom
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