From maechler at stat.math.ethz.ch  Fri Mar 16 09:32:12 2007
From: maechler at stat.math.ethz.ch (Martin Maechler)
Date: Fri, 16 Mar 2007 09:32:12 +0100
Subject: [Rd] R 2.5.0 devel try issue in conjuntion with S4 method
	dispatch
In-Reply-To: <45FA428C.9030402@biostat.ku.dk>
References: <45F98B1D.2060501@epigenomics.com> <m2ejnq88nn.fsf@ziti.local>
	<45FA428C.9030402@biostat.ku.dk>
Message-ID: <17914.22028.162669.917442@stat.math.ethz.ch>


>>>>> "PD" == Peter Dalgaard <p.dalgaard at biostat.ku.dk>
>>>>>     on Fri, 16 Mar 2007 08:09:00 +0100 writes:

    PD> Seth Falcon wrote:
    >> ml-it-r-devel at epigenomics.com writes:
    >> 
    >>> Investigating this new implementation I come across an issue in
    >>> conjuntion with using S4 classes and methods. try(expr) does not return an
    >>> object with attribute 'try-error' in case of method dispatch failure
    >>> in the wrapped expression which to me seems not
    >>> quite correct.
    >>> 
    >> 
    >> We've seen some similar issues but had not had time to track them
    >> down.
    >> 
    >> 
    >>> Examples to reproduce the observation:
    >>> 
    >> 
    >> It isn't S4 specific.  The issue seems more about anonymous
    >> calls/functions.
    >> 
    >> ll = list()
    >> ll[[1]] = function(x) stop("died")
    >> 
    >> v = try(do.call(ll[[1]], list(1)), silent=TRUE)
    >> Error in as.list(call)[[1]] == "doTryCatch" : 
    >> comparison (1) is possible only for atomic and list types
    >> > v
    >> Error: object "v" not found
    >> 
    >> I don't fully understand why the call is being computed, but the
    >> following seems to get things going.
    >> 
    >> + seth
    >> 
    >> --- a/src/library/base/R/New-Internal.R
    >> +++ b/src/library/base/R/New-Internal.R
    >> @@ -7,7 +7,8 @@ try <- function(expr, silent = FALSE) {
    >> ## Patch up the call to produce nicer result for testing as
    >> ## try(stop(...)).  This will need adjusting if the
    >> ## implementation of tryCatch changes.
    >> -            if (as.list(call)[[1]] == "doTryCatch")
    >> +            callFun <- as.list(call)[[1]]
    >> +            if (is.name(callFun) && callFun == "doTryCatch")
    >> call <- sys.call(-4)
    >> dcall <- deparse(call)[1]
    >> prefix <- paste("Error in", dcall, ": ")
    >> 
    >> 
    PD> Good catch, Seth. 

yes, indeed!

    PD> The code still looks a bit clunky though, and I wonder 
    PD> if this wouldn't be nicer:

    PD> if (identical(call[[1]], quote(doTryCatch))) call <- sys.call(-4)

    PD> I.e., the thing that is clearly wrong is the use of "==" on something 
    PD> that is not necessarily a simple name, but the use of as.list seems 
    PD> unnecessary and comparisons between strings and names is a bit awkward too.

Indeed, I had similar thoughts when reading the part of code
Seth was patching 
{ but would have used the old-fashioned   as.name("doTryCatch")
  instead of the modern  quote(doTryCatch)
  for the only reason that I'm probably slightly ``older fashioned''
  than Peter  ;-)  
}

Martin



[Rd] R 2.5.0 devel try issue in conjuntion with S4 method dispatch
Martin Maechler