[Bioc-devel] Nasty function collision

[Seth Falcon]

Martin Maechler <maechler at stat.math.ethz.ch> writes:

>     JimMcD> Seth Falcon wrote:
>     >> Code that does the optional generic definition really needs to go away
>     >> at this point (I know it was needed at some point, so I'm not trying
>     >> to point a finger or anything).  To be specific, I'm referring to any
>     >> code that does something along the lines of:
>     >> 
>     >> if (!isGeneric("foo"))
>     >> setGeneric("foo") ...
>
> Hmm, am I understanding correctly?
> This is "bad" because it can destroy consistent behavior in the
> case when there's already a non-S4 function foo() present?

Uh, here's where I'm confused.  The check isGeneric("foo") tells you,
AFAIK, if foo is an S4 generic, not whether there is a foo function
already defined that could "volunteer" to be the default method for a
new generic.  Am I missing something?

> Whereas  when not doing setGeneric(), the non-S4 function foo() will become
> the default method as soon as the first setMethod("foo", ....)
> happens?
> But if that's true, it seems to me, setGeneric() should never be
> used anymore, and I somehow doubt that..

I don't think I understand what you are getting at.  So let me explain
further my concern about the following coding idiom:

     if (!isGeneric("foo"))
         setGeneric("foo", ...)

If you put this code in your package, you are saying, "Someone else
somewhere might have defined a generic and I will use it if I can find
it".  And I think this makes _no_ sense.  How would one know what the
formal arguments are, for example?  [the common idion is to next
define a generic without any introspection of the possibly
pre-existing generic].

It turns out that this isn't as dangerous as it could be because
packages with S4 code use SaveImage: Yes.  This means the package code
isn't executed at load time so it won't get confused, for example, by
a call to setGeneric at the global level prior to package loading.
But it also means the code is pointless.  If you can't pick up any old
generic out there, then you can _only_ pickup ones you should know
about.  Therefore, you should either know it is there and use it or
define your own.  I just don't understand the conditional logic.

IMO, you should either define your own generic or attach a method to a
_specific_ and _known_ generic defined elsewhere.  And for the latter,
I really like the recent changes that allow you to very clearly
specify the generic you are talking about:

    setMethod(Biobase::exprs, ...)

> I tend to agree with Jim here that I think it's a pretty
> ``impolite'' inter-package-citizenship behavior to ever define
>
>   setMethod("SomeGeneric", "ANY", function(......) ......)
>
> because you do trash other method definitions for
> "SomeGeneric" at your package load time.

It is perfectly fine behavior, IMO, as long as SomeGeneric is defined
in the package's name space.  [I'm not going to press this argument
too far; It seems like an odd way to code, but I haven't looked
closely and there may be a good use case for it].

> I'd say a good R-izen has at least one of his/her own classes in
> every signature (s)he defines methods for.

I'm pretty sure that is bogus.  If I define a plain old function, by definition
it has signature "ANY" and that doesn't bother you, does it?  If I
define my own generic and it has a defualt method, then you get
behavior for "ANY"...  And to be more particular, it is _perfectly_
reasonable to define methods for other peoples classes (but I bet that
isn't what you meant in the first place ;-)!

+ seth, FHCRC Seattle