[Rd] A Design Error (Re: S4 objects for S3 methods)

[Yohan Chalabi]

>>>> "JC" == John Chambers <jmc at r-project.org>
>>>> on Mon, 09 Mar 2009 09:53:06 -0700

   JC> As Yohan points out, and as we found in testing CRAN packages,
   JC> there are
   JC> a number of examples where programmers have written S3 methods
   JC> for S4
   JC> classes, such as print.aTest() below.
   JC>
   JC> This may well have seemed an easy and convenient mechanism,
   JC> but it is a
   JC> design error with potentially disastrous consequences.
   JC> Note that this
   JC> is fundamentally different from an S3 method defined for an
   JC> S3 class and
   JC> inherited by an S4 class that extends the S3 class.
   JC>
   JC> We haven't decided whether to re-enable this, but if so it
   JC> will be with
   JC> a warning at user call and/or at package check time.
   JC>
   JC> Please turn such methods into legitimate S4 methods.
   JC> Usually the change
   JC> is a simple call to setMethod(); in some cases, such as this
   JC> example you
   JC> may need a bit more work, such as a show() method to call
   JC> the print.*
   JC> function.
   JC>
   JC> DETAILS:
   JC>
   JC> There are at least two serious flaws in this mechanism, plus
   JC> some minor
   JC> defects.
   JC>
   JC> 1. S3 dispatch does not know about S4 class inheritance.
   JC> So if as a user of Yohan's code, for example, I define a class
   JC> setClass(bTest, contains = aTest)
   JC> then that class will not inherit any of the S3 methods.
   JC> In the case of
   JC> print, that will be obvious.  The disaster waiting to happen
   JC> is when the
   JC> method involves numerical results, in which case I may be
   JC> getting
   JC> erroneous results, with no hint of the problem.
   JC>
   JC> 2. Conversely, S4 dispatch does not know about the S3 method.
   JC> So, if my new class was:
   JC> setClass(bTest, contains = c(aTest, waffle7)
   JC> suppose waffle7 has some huge inheritance, in the midst of
   JC> which is a
   JC> method for a generic function of importance.  I expect to
   JC> inherit the
   JC> method for aTest because it's for a direct superclass, but I
   JC> won't, no
   JC> matter how far up the tree the other method occurs.  (Even if
   JC> point 1
   JC> were fixed)
   JC>
   JC> Again, this would be obvious for a print method, but potentially
   JC> disastrous elsewhere.
   JC>
   JC> There are other minor issues, such as efficiency: the S3 method
   JC> requires two dispatches and perhaps may do some extra copying.
   JC> But 1
   JC> and 2 are the killers.
   JC>
   JC> Just to anticipate a suggestion: Yes, we could perhaps fix 1
   JC> by adding
   JC> code to the S3 dispatch, but this would ambiguate the legitimate
   JC> attempt
   JC> to handle inherited valid S3 methods correctly.
   JC>
   JC> John

Dear John, 

Thank you for the detailed explanation.

I completely understand that it is a design error and that it should be
fixed.  As you said it is a matter of using 'setMethod'. 

My main concern is that such a change happens one month before a new
release. This is very little time for developers to make their packages
consistent with the new release.

As a side note, I have noticed that it is still possible to define S3
methods for S4 classes which do not contains a super-class like matrix.
But the disastrous consequences that you explained would still be
possible in my opinion.

setClass("aTest", representation(.Data = "matrix", comment = "character"))
a <- new("aTest", .Data = matrix(1:4, ncol = 2), comment = "aTest")
as.matrix.aTest <- function(x, ...) getDataPart(x)
as.matrix(a) # returns same S4 object

# but
setClass("bTest", representation(Data = "matrix", comment = "character"))
b <- new("bTest", Data = matrix(1:4, ncol = 2), comment = "hello")
as.matrix.bTest <- function(x, ...) b at Data
as.matrix(b) # does work

Are you planing to turn this off too?

Again, I understand that developers should conform with the S4 style
but a one month notice is very short in my opinion. Moreover adding a
warning in R CMD check would be the same because developer won't be
able to submit packages since CRAN only accepts packages which pass R
CMD check without warnings.

Best regards,
Yohan



-- 
PhD student
Swiss Federal Institute of Technology
Zurich

www.ethz.ch