From maechler at stat.math.ethz.ch  Sat Jun 13 13:35:41 2015
From: maechler at stat.math.ethz.ch (Martin Maechler)
Date: Sat, 13 Jun 2015 13:35:41 +0200
Subject: [Rd] RFC: Declaring "foo.bar" as nonS3method() ?!
In-Reply-To: <alpine.LFD.2.11.1506121023390.1461@itasca.stat.uiowa.edu>
References: <000e01d0a387$885ad130$99107390$@mcmaster.ca>
	<21882.36966.429889.227076@stat.math.ethz.ch>
	<21882.37975.726216.982364@stat.math.ethz.ch>
	<557ABD04.1040205@gmail.com>
	<21882.49031.599800.412740@fangorn.hornik.net>
	<557AC6FC.8060206@gmail.com>
	<21882.51223.144401.149543@fangorn.hornik.net>
	<CABdHhvFTb=erfo6T+83pBqG5b0y0hQLgO3u=Ywh6F2WoZu8crw@mail.gmail.com>
	<557AF6E7.2040805@gmail.com>
	<alpine.LFD.2.11.1506121023390.1461@itasca.stat.uiowa.edu>
Message-ID: <21884.5517.93162.167363@stat.math.ethz.ch>

>>>>> Luke Tierney <luke-tierney at uiowa.edu>
>>>>>     on Fri, 12 Jun 2015 10:30:29 -0500 writes:

    > The notes available off the devloper page
    > https://developer.r-project.org/ describe some of the rationale for
    > the S3 method search design. One thing that has changed since then is
    > that all packages now have name spaces. We could change the search
    > algorithm to skip attached package exports (and package imports and
    > base), which would require methods defined in packages that are to be
    > accessible outside the package to be declared.  Methods defined inside
    > a package for internal use or methods defined in scripts not in
    > packages would still be found. Packages not currently registering
    > their methods would have to do so -- not sure how many that would
    > affect. Testing on CRAN/Bioc should show how much of an effect this
    > would have and whether there are any other issues.

    > Best,
    > luke

Thanks a lot Luke, for the extra perspective.  
I think the four R core commenters here (Duncan, Kurt, Luke and
me) agree that this is not trivial to implement, but hopefully
not too hard either, and I think we also +- agree that it seems
desirable to try adding a bit more flexibility in how functions
are "made into" S3 methods. 

I had not envisaged to change the S3 method search
algorithm but rather to tweak part of it "database" but am aware
that I don't know enough of the details.
Also, I did not find which notes (from developer.r-project.org)
you were refering to.

Given the broad agreement that we want to start working /
investigating this,  we can well close the thread here for now
(and deal with ideas, issues, details within R-core).

Martin

    > On Fri, 12 Jun 2015, Duncan Murdoch wrote:

    >> On 12/06/2015 10:53 AM, Hadley Wickham wrote:
    >>> To me, it seems like there's actually two problems here:
    >>> 
    >>> 1) Preventing all() from dispatching to all.effects() for objects of
    >>> class effects
    >>> 2) Eliminating the NOTE in R CMD check
    >>> 
    >>> My impression is that 1) actually causes few problems, particularly
    >>> since people are mostly now aware of the problem and avoid using `.`
    >>> in function names unless they're S3 methods. Fixing this issue seems
    >>> like it would be a lot of work for relatively little gain.
    >>> 
    >>> However, I think we want to prevent people from writing new functions
    >>> with this confusing naming scheme, but equally we want to grandfather
    >>> in existing functions, because renaming them all would be a lot of
    >>> work (I'm looking at you t.test()!).
    >>> 
    >>> Could we have a system similar to globalVariables() where you could
    >>> flag a function as definitely not being an S3 method? I'm not sure
    >>> what R CMD check should do - ideally you wouldn't be allow to use
    >>> method.class for new functions, but still be able suppress the note
    >>> for old functions that can't easily be changed.
    >> 
    >> We have a mechanism for suppressing the warning for existing functions,
    >> it's just not available to users to modify.  So it would be possible to
    >> add effects::all.effects to the stop list, and this might be the easiest
    >> action here.
    >> 
    >> This isn't perfect because all.effects() would still act as a method.
    >> However,  it does give the deprecated message if you ever call it, so
    >> nobody would do this unknowingly.  The only real risk is that if anyone
    >> ever wrote an all.effects function that *was* supposed to be an S3
    >> method, it might be masked by the one in effects.
    >> 
    >> Duncan Murdoch
    >> 
    >>> 
    >>> Hadley
    >>> 
    >>> On Fri, Jun 12, 2015 at 6:52 AM, Kurt Hornik <Kurt.Hornik at wu.ac.at> wrote:
    >>>>>>>>> Duncan Murdoch writes:
    >>>> 
    >>>>> On 12/06/2015 7:16 AM, Kurt Hornik wrote:
    >>>>>>>>>>> Duncan Murdoch writes:
    >>>>>> 
    >>>>>>> On 12/06/2015 4:12 AM, Martin Maechler wrote:
    >>>>>>>> This is a topic ' "apparent S3 methods" note in R CMD check '
    >>>>>>>> from R-package-devel
    >>>>>>>> https://stat.ethz.ch/pipermail/r-package-devel/2015q2/000126.html
    >>>>>>>> 
    >>>>>>>> which is relevant to here because some of us have been thinking
    >>>>>>>> about extending R  because of the issue.
    >>>>>>>> 
    >>>>>>>> John Fox, maintainer of the 'effects' package has enquired about
    >>>>>>>> the following  output from  'R CMD check effects'
    >>>>>>>> 
    >>>>>>>>> * checking S3 generic/method consistency ... NOTE
    >>>>>>>>> Found the following apparent S3 methods exported but not registered:
    >>>>>>>>> all.effects
    >>>>>>>> 
    >>>>>>>> and added
    >>>>>>>> 
    >>>>>>>>> The offending function, all.effects(), is deprecated in favour of
    >>>>>>>>> allEffects(), but I'd rather not get rid of it for backwards compatibility.
    >>>>>>>>> Is there any way to suppress the note without removing all.effects()?
    >>>>>>>> 
    >>>>>>>> and I had agreed that this was a "False Positive" in this case.
    >>>>>>>> 
    >>>>>>>> [.......]
    >>>>>>>> 
    >>>>>>>> and then
    >>>>>>>> 
    >>>>>>>>> Now I agree .. and have e-talked about this with another R core
    >>>>>>>>> member .. that it would be desirable for the package author to
    >>>>>>>>> effectively declare the fact that such a function is not an S3
    >>>>>>>>> method even though it "looks like it" at least if looked from far.
    >>>>>>>> 
    >>>>>>>>> So, ideally, you could have something like
    >>>>>>>> 
    >>>>>>>>> nonS3method("all.effects")
    >>>>>>>> 
    >>>>>>>>> somewhere in your package source ( in NAMESPACE or R/*.R )
    >>>>>>>>> which would tell the package-checking code -- but *ALSO* all the other S3
    >>>>>>>>> method code that  all.effects should be treated as a regular R
    >>>>>>>>> function.
    >>>>>>>> 
    >>>>>>>>> I would very much like such a feature in R, and for that reason,
    >>>>>>>>> I'm cross posting this (as one of the famous exceptions that
    >>>>>>>>> accompany real-life rules!!) to R-devel.
    >>>>>>>> 
    >>>>>>>> and actually I did *not* cross post, but have now moved the
    >>>>>>>> relevant part of the thread to  R-devel.
    >>>>>>>> 
    >>>>>> 
    >>>>>>> It sounds like a good idea.  It's a nontrivial amount of work, because
    >>>>>>> of the "all the other S3 method code" part.  There's the question of
    >>>>>>> functions defined outside of packages:  presumably they are still S3
    >>>>>>> methods, with no way to suppress that.
    >>>>>> 
>>>>> I am not sure this is the right solution: S3 dispatch will still occur
>>>>> because we first look at foo.bar exports and then in the S3 registry,
>>>>> afaicr (the "all the other S3 method code" part).
    >>>>>> 
>>>>> If we could move to only looking at the registry for dispatch, there
>>>>> would be no need to declare situations where we should not dispatch on
>>>>> foo.bar exports.
    >>>>>> 
    >>>> 
    >>>>> I think that would break a lot of existing scripts.  I think the logic
    >>>>> should be something like this.
    >>>> 
    >>>>> For each class in the class list:
    >>>> 
    >>>>> Search backwards through the environment chain.  If the current location
    >>>>> is a package environment or namespace, look only in the registry.  If
    >>>>> not, look at all functions.
    >>>> 
    >>>>> If that search failed, try the next class.
    >>>> 
    >>>> Yep---that's what I meant.  I forgot to write the "if namespace
    >>>> semantics applies" part :-)
    >>>> 
    >>>> Best
    >>>> -k
    >>>> 
    >>>>> A variation on the test is:  If there's a registry in the current
    >>>>> location, look there.  But I think the registry is not on the search
    >>>>> list, so I'm not sure that would work.
    >>>> 
    >>>>> This assumes that we keep separate registries in each package; if we
    >>>>> merge them into one big registry, it gets harder.  I'm not familiar
    >>>>> enough with the code to know which way we do it.
    >>>> 
    >>>>> Duncan Murdoch
    >>>> 
    >>>> ______________________________________________
    >>>> R-devel at r-project.org mailing list
    >>>> https://stat.ethz.ch/mailman/listinfo/r-devel
    >>> 
    >>> 
    >>> 
    >> 
    >> ______________________________________________
    >> R-devel at r-project.org mailing list
    >> https://stat.ethz.ch/mailman/listinfo/r-devel
    >> 

    > -- 
    > Luke Tierney
    > Ralph E. Wareham Professor of Mathematical Sciences
    > University of Iowa                  Phone:             319-335-3386
    > Department of Statistics and        Fax:               319-335-3017
    > Actuarial Science
    > 241 Schaeffer Hall                  email:   luke-tierney at uiowa.edu
    > Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu



[Rd] RFC: Declaring "foo.bar" as nonS3method() ?!
Martin Maechler