Mario Annau <mario.annau at gmail.com>
on Wed, 11 Feb 2015 20:18:53 +0100 writes:
> sorry - I just got irritated by my different R-versions.
> The behaviour I described in the previous mail was discovered using R
> 3.1.2 without bind_activation(TRUE). In r67773 all calls are delegated
> to r/cbind.matrix and not r/cbind2.
> As a workaround I have now implemented an S3 method for my S4 class
> which correctly dispatches for both versions (3.1.2 and r67699+) - see
> also the commit for the h5 package on github:
> https://github.com/mannau/h5/commit/20daea37ade1a317458c8a1d03928f579e457f93.
> Any better ideas are welcome.
and in the mean time there have been a few off-list e-mails,
{"No, using an S3 method was definitely not the idea of
Michael's changes!" .. }
and many hours of work by me.
R-devel svn rev 67852 and later now has cbind() / rbind()
working in a better way, dipatching to either cbind2(), rbind2()
S4 methods for "your" classes, or to S4 rbind() or cbind()
methods for your classes.
Notably the new code now should create column / rownames
analogously to base::cbind / rbind, influenced by deparse.level
in the case of non-matrix arguments.
Small changes in some outputs may occur, notably as the hidden
methods:::cbind and rbind functions (think of "S4 default method")
now do obey deparse.level and also otherwise should create row
and column names in the same way as base::[cr]bind().
Martin Maechler
ETH Zurich and R Core Team
> Am 09/02/15 um 23:38 schrieb Michael Lawrence:
>> Are you able to create a reproducible example, somehow?
>>
>> Thanks,
>> Michael
>>
>> On Mon, Feb 9, 2015 at 2:28 PM, Mario Annau <mario.annau at gmail.com
>> <mailto:mario.annau at gmail.com>> wrote:
>>
>> Hi Michael,
>> I've tested your change in r67699 (using r67773) and the function now
>> correctly dispatches to r/cbind2 within the R-session without
>> bind_activation(TRUE). However, running unit tests using R CMD check I
>> figured out that the same function call delegates to r/cbind.matrix
>> (function uses S4 class as first- and matrix as second argument). Is
>> this a bug and/or how can I get function dispatch right (to r/cbind2)
>> for my test cases?
>> best,
>> mario
>>
>>
>> Am 02/02/15 um 12:32 schrieb Martin Maechler:
>> >>>>>> Michael Lawrence <lawrence.michael at gene.com
>> <mailto:lawrence.michael at gene.com>>
>> >>>>>> on Sun, 1 Feb 2015 19:23:06 -0800 writes:
>> > > I've implemented the proposed changes in
>> > > R-devel. Minimally tested, so please try it. It should
>> > > delegate to r/cbind2 when there is at least one S4
>> > > argument and S3 dispatch fails (so you'll probably want to
>> > > add an S3 method for your class to introduce a conflict,
>> > > otherwise it will dispatch to cbind.data.frame if one of
>> > > the args is a data.frame). There may no longer be a need
>> > > for cBind() and rBind().
>> >
>> > This sounds great! Thank you very much, Michael!
>> > :-) :-)
>> >
>> > ... but .... :-( experiments with the Matrix package (and R
>> > devel with your change), show a remaining buglet with treating of
>> > > M1 <- Matrix(m1 <- matrix(1:12, 3,4))
>> > > cbind(m1, MM = -1)
>> > MM
>> > [1,] 1 4 7 10 -1
>> > [2,] 2 5 8 11 -1
>> > [3,] 3 6 9 12 -1
>> > > cbind(M1, MM = -1) ## ---- notice the "..."
>> > 3 x 5 Matrix of class "dgeMatrix"
>> > ...
>> > [1,] 1 4 7 10 -1
>> > [2,] 2 5 8 11 -1
>> > [3,] 3 6 9 12 -1
>> > > rbind(R1 = 10:11, m1)
>> > [,1] [,2] [,3] [,4]
>> > R1 10 11 10 11
>> > 1 4 7 10
>> > 2 5 8 11
>> > 3 6 9 12
>> > > rbind(R1 = 10:11, M1) ## --- notice the 'deparse.level'
>> > 4 x 4 Matrix of class "dgeMatrix"
>> > [,1] [,2] [,3] [,4]
>> > deparse.level 10 11 10 11
>> > 1 4 7 10
>> > 2 5 8 11
>> > 3 6 9 12
>> >
>> > Also, it seems you are not observing the 'deparse.level'
>> > argument at all:
>> > Looking at the last three lines of the example in ?cbind,
>> >
>> > rbind(1:4, c = 2, "a++" = 10, dd, deparse.level = 0) # middle
>> > rbind(1:4, c = 2, "a++" = 10, dd, deparse.level = 1) # 3
>> > rbind(1:4, c = 2, "a++" = 10, dd, deparse.level = 2) # 4 rownames
>> >
>> > but using a Matrix matrix 'dd', we see that (row)names
>> > construction needs to amended:
>> >
>> > > (dd <- Matrix(rbind(c(0:1,0,0))))
>> > 1 x 4 sparse Matrix of class "dgCMatrix"
>> >
>> > [1,] . 1 . .
>> >
>> > > rbind(1:4, c = 2, "a++" = 10, dd, deparse.level = 0) # middle
>> > 4 x 4 sparse Matrix of class "dgCMatrix"
>> >
>> > deparse.level 1 2 3 4
>> > c 2 2 2 2
>> > a++ 10 10 10 10
>> > . 1 . .
>> > > rbind(1:4, c = 2, "a++" = 10, dd, deparse.level = 1) # 3
>> > 4 x 4 sparse Matrix of class "dgCMatrix"
>> >
>> > deparse.level 1 2 3 4
>> > c 2 2 2 2
>> > a++ 10 10 10 10
>> > . 1 . .
>> > > rbind(1:4, c = 2, "a++" = 10, dd, deparse.level = 2) # 4 rownames
>> > 4 x 4 sparse Matrix of class "dgCMatrix"
>> >
>> > deparse.level 1 2 3 4
>> > c 2 2 2 2
>> > a++ 10 10 10 10
>> > . 1 . .
>> > > On Mon, Jan 26, 2015 at 3:55 AM, Martin Maechler <
>> > > maechler at lynne.stat.math.ethz.ch
>> <mailto:maechler at lynne.stat.math.ethz.ch>> wrote:
>> > >> >>>>> Michael Lawrence <lawrence.michael at gene.com
>> <mailto:lawrence.michael at gene.com>> >>>>>
>> > >> on Sat, 24 Jan 2015 06:39:37 -0800 writes:
>> > >>
>> > >> > On Sat, Jan 24, 2015 at 12:58 AM, Mario Annau >
>> > >> <mario.annau at gmail.com <mailto:mario.annau at gmail.com>>
>> wrote: >> Hi all, this question
>> > >> has already been posted on >> stackoverflow, however
>> > >> without success, see also
>> > >> >>
>> > >> >> I have written a package using S4 classes and would
>> > >> like >> to use the functions rbind, cbind with these
>> > >> defined >> classes.
>> > >> >>
>> > >> >> Since it does not seem to be possible to define rbind
>> > >> and >> cbind directly as S4 methods (see ?cBind) I
>> > >> defined >> rbind2 and cbind2 instead:
>> > >> > This needs some clarification. It certainly is possible
>> > >> to > define cbind and rbind methods. The BiocGenerics
>> > >> package > defines generics for those and many methods are
>> > >> defined by > e.g. S4Vectors, IRanges, etc. The issue is
>> > >> that dispatch > on "..." is singular, i.e., you can only
>> > >> specify one class > that all args in "..." must share
>> > >> (potentially through > inheritance).
>> > >>
>> > >> > Thus, trying to combine objects from a > different
>> > >> hierarchy (or non-S4 objects) will not > work.
>> > >>
>> > >> Yes, indeed, that's the drawback
>> > >>
>> > >> I've been there almost surely before everyone else, with
>> > >> the Matrix package... and I have been the author of
>> > >> cbind2(), rbind2(), and of course, of cBind(), and
>> > >> rBind().
>> > >>
>> > >> At the time when I introduced these, the above
>> > >> possibility of writing S4 methods for '...' where not
>> > >> yet part of R.
>> > >>
>> > >> > This has not been a huge problem for us in >
>> > >> practice. For example, we have a DataFrame object that >
>> > >> mimics data.frame. To cbind a data.frame with a
>> > >> DataFrame, > the user can just call the DataFrame() >
>> > >> constructor. rbind() between different data structures is
>> > >> > much less common.
>> > >>
>> > >> well... yes and no. Think of using the Matrix package,
>> > >> maybe with another package that defines another
>> > >> generalized matrix class... It would be nice if things
>> > >> worked automatically / perfectly there.
>> > >>
>> > >> > The cBind and rBind functions in Matrix (and the
>> > >> r/cbind > that get installed by bind_activation, the code
>> > >> is shared) > work by recursing, dropping the first
>> > >> argument until two > are left, and then combining with
>> > >> r/cbind2(). The Biobase > package uses a similar strategy
>> > >> to mimic c() via its > non-standard combine()
>> > >> generic. The nice thing about the > combine() approach is
>> > >> the user entry point and the generic > are the same,
>> > >> instead of having methods on rbind2() and > the user
>> > >> calling rBind().
>> > >>
>> > >> > I would argue that bind_activation(TRUE) should be >
>> > >> discouraged,
>> > >>
>> > >> Yes, you are right Michael; it should be discouraged at
>> > >> least to be run in a *package*. One could think of its
>> > >> use by an explicit user call.
>> > >>
>> > >> > because it replaces the native rbind and > cbind with
>> > >> recursive variants that are going to cause > problems,
>> > >> performance and otherwise. This is why it is >
>> > >> hidden. Perhaps a reasonable compromise would be for the
>> > >> > native cbind and rbind to check whether any arguments
>> > >> are > S4 and if so, resort to recursion. Recursion does
>> > >> seem to > be a clean way to implement "type promotion",
>> > >> i.e., to > answer the question "which type should the
>> > >> result be when > faced with mixed-type args?".
>> > >>
>> > >> Exactly. That has been my idea at the time .. ((yes,
>> > >> I'm also the author of the bind_activation()
>> > >> "(mis)functionality".))
>> > >>
>> > >> > Hopefully others have better ideas.
>> > >>
>> > >> that would be great.
>> > >>
>> > >> And even if not, it would be great if we could implement
>> > >> your idea > Perhaps a reasonable compromise would be for
>> > >> the > native cbind and rbind to check whether any
>> > >> arguments are > S4 and if so, resort to recursion.
>> > >>
>> > >> without a noticable performance penalty in the case of no
>> > >> S4 arguments.
>> > >>
>> > >> Martin
>> > >>
>> > >>
>> > >> >> setMethod("rbind2", signature(x="ClassA", y = "ANY"),
>> > >> >> function(x, y) { # Do stuff ... })
>> > >> >>
>> > >> >> setMethod("cbind2", signature(x="ClassA", y = "ANY"),
>> > >> >> function(x, y) { # Do stuff ... })
>> > >> >>
>> > >> >> >From ?cbind2 I learned that these functions need to
>> > >> be >> activated using methods:::bind_activation to
>> > >> replace >> rbind and cbind from base.
>> > >> >>
>> > >> >> I included the call in the package file R/zzz.R using
>> > >> the >> .onLoad function:
>> > >> >>
>> > >> >> .onLoad <- function(...) { # Bind activation of
>> > >> cbind(2) >> and rbind(2) for S4 classes >>
>> > >> methods:::bind_activation(TRUE) } This works as >>
>> > >> expected. However, running R CMD check I am now getting
>> > >> >> the following NOTE since I am using an unexported >>
>> > >> function in methods:
>> > >> >>
>> > >> >> * checking dependencies in R code ... NOTE Unexported
>> > >> >> object imported by a ':::' call: >>
>> > >> 'methods:::bind_activation' See the note in ?`:::` about
>> > >> >> the use of this operator. How can I get rid of the
>> > >> NOTE >> and what is the proper way to define the methods
>> > >> cbind >> and rbind for S4 classes in a package?
>> > >> >>
>> > >> >> Best, mario
>> > >> >>
>> > >> >> ______________________________________________ >>
>> > >> R-devel at r-project.org <mailto:R-devel at r-project.org>
>> > >> > ______________________________________________ >
>> > >> R-devel at r-project.org <mailto:R-devel at r-project.org>