Bug 16141 - In some context, callGeneric() leaves some arguments behind
Summary: In some context, callGeneric() leaves some arguments behind
Status: CLOSED FIXED
Alias: None
Product: R
Classification: Unclassified
Component: S4methods (show other bugs)
Version: R-devel (trunk)
Hardware: All All
: P5 normal
Assignee: R-core
URL:
Depends on:
Blocks:
 
Reported: 2015-01-08 00:32 UTC by Hervé Pagès
Modified: 2015-12-14 13:45 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hervé Pagès 2015-01-08 00:32:44 UTC
In some context, callGeneric() (called with no arg) isn't able to collect and pass all the arguments that were passed to the method. Here is a (totally artificial) example:

  setGeneric("foo", function(x, ...) standardGeneric("foo"))
  setMethod("foo", "character",
      function(x, print.it=FALSE) if (print.it) print(x))

  foo("Hi!", print.it=TRUE)  # Hi!

  setMethod("foo", "numeric",
    function(x, print.it=FALSE) {
        x <- as.character(x)
        callGeneric()
    }
  )

  foo(335, print.it=TRUE)  # 335

First problem:

  toto1 <- function(x, ...) foo(x, ...)
  toto1("Hi!", print.it=TRUE)  # Hi!
  toto1(335, print.it=TRUE)    # I get nothing here!

Second problem:

  toto2 <- function(...) foo(...)
  toto2("Hi!", print.it=TRUE)  # Hi!
  toto2(335, print.it=TRUE)    # I get an error here! (unable to find an
                               # inherited method for function 'foo' for
                               # signature '"missing"')

My workaround right now is to explicitly specify all the arguments in the call to callGeneric():

  setMethod("foo", "numeric",
    function(x, print.it=FALSE) {
        x <- as.character(x)
        callGeneric(x, print.it=print.it)
    }
  )

Can be tedious and hurt readability when there are many arguments, so I collect them all with:

  setMethod("foo", "numeric",
    function(x, print.it=FALSE) {
        x <- as.character(x)
        do.call(callGeneric, as.list(match.call()[-1L]))
    }
  )

There is probably a better way but that seems to work. I wish callGeneric() could just do that for me.

  > sessionInfo()
  R Under development (unstable) (2014-10-27 r66886)
  Platform: x86_64-unknown-linux-gnu (64-bit)
  ...

Thanks,
H.
Comment 1 John Chambers 2015-01-08 01:11:00 UTC
The problem is in the combination of callGeneric() and a method that has different formal arguments than the generic.

Strictly, methods and generic must have identical arguments, since arguments are not rematched on dispatch.  We relax this by a kludge that redefines the method as a local function, with the actual method being generated with the "correct" arguments, containing a call to the local function.

This mechanism confuses, at least sometimes, the handling of callGeneric() with no arguments.

Unless someone wants to re-implement the approach to non-standard arguments in methods, we should probably just warn people in the documentation that they must use identical arguments in the method if they want to use callGeneric() with no arguments.
Comment 2 Michael Lawrence 2015-01-08 03:23:05 UTC
I will take a stab at fixing this. If we add the proposed "envir" argument to match.call, we should be able to at least capture the dots.
Comment 3 Hervé Pagès 2015-01-08 19:16:49 UTC
(In reply to John Chambers from comment #1)
> The problem is in the combination of callGeneric() and a method that has
> different formal arguments than the generic.
> 
> Strictly, methods and generic must have identical arguments, since arguments
> are not rematched on dispatch.  We relax this by a kludge that redefines the
> method as a local function, with the actual method being generated with the
> "correct" arguments, containing a call to the local function.
> 
> This mechanism confuses, at least sometimes, the handling of callGeneric()
> with no arguments.
> 
> Unless someone wants to re-implement the approach to non-standard arguments
> in methods, we should probably just warn people in the documentation that
> they must use identical arguments in the method if they want to use
> callGeneric() with no arguments.

Or someone could fix callGeneric() to work with the .local kludge.

Or maybe the .local kludge could sometimes (if not most of the times) be avoided. It seems that when the arguments of the method and generic are not identical, most of the time they're sufficiently close to allow easy rematching of (some of the) arguments on dispatch. Like in the following situation:

  Generic: ... is the last formal argument e.g. (X, Y, ...)

  Method: Additional arguments are inserted right before ... e.g.
          (X, Y, a, b, c, ...) or they're just replacing ... e.g.
          (X, Y, a, b, c).
          It guess these are the only 2 reasonable ways to add arguments
          to the generic right? (Note that it seems possible to do crazy
          things like (X, a, b, Y, c, ...) but this breaks the .local kludge.
          Anyway why would anybody do this...)

Note that this is a *very* common situation(*) and it feels like a natural thing to do from a developper point of view. It is much better IMO than keeping identical formals and manually collecting the extra arguments inside the method. It makes the code simpler, cleaner, easier to read. It's also a big plus from a user point of view because the extra args are fully visible i.e. they appear in the \usage section of the man page for the method, they have their own separate entries in the \arguments section, and they can be programmatically discovered by calling args() on the result of selectMethod(). The only drawback at the moment is that doing so comes with the .local kludge. 

So my question is: isn't this a situation where the .local kludge could be avoided by revisiting the "no rematching of the arguments on dispatch" dogma?

AFAIR the .local kludge has always been a source of confusion and frustration amongst BioC developpers. A typicial situation where it's known to cause problems is when the code inside the method needs to access the environment from which the generic function was called. This happens for example in some of the "with" and "subset" methods we have (both of them are S4 generics in Bioconductor). I think Michael was able to use some nice tricks recently to address this. Maybe the same kind of tricks can be used to make callGeneric() work with the .local kludge. Don't know, he's the non-standard evaluation expert.

Thanks,
H.

(*) We may have 100+ methods in BioC that add arguments to the generic,
    and I suspect that all of them do it *that* way.
Comment 4 Hervé Pagès 2015-01-08 19:18:06 UTC
(In reply to Michael Lawrence from comment #2)
> I will take a stab at fixing this. If we add the proposed "envir" argument
> to match.call, we should be able to at least capture the dots.

Thanks Michael.
Comment 5 Michael Lawrence 2015-01-14 15:44:30 UTC
Should be fixed as of r67467.
Comment 6 Hervé Pagès 2015-01-20 17:58:29 UTC
(In reply to Michael Lawrence from comment #5)
> Should be fixed as of r67467.

Thanks Michael. I'll give this a try after I managed to update my R.

H.