From jfox at mcmaster.ca  Sat Jun 13 14:52:14 2015
From: jfox at mcmaster.ca (John Fox)
Date: Sat, 13 Jun 2015 08:52:14 -0400
Subject: [Rd] RFC: Declaring "foo.bar" as nonS3method() ?!
In-Reply-To: <21884.5517.93162.167363@stat.math.ethz.ch>
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>
	<21884.5517.93162.167363@stat.math.ethz.ch>
Message-ID: <web-562948601@cgpsrv2.cis.mcmaster.ca>

Dear Martin et al.,

Thank you for considering so thoroughly the issue that I raised.

Best,
 John

On Sat, 13 Jun 2015 13:35:41 +0200
 Martin Maechler <maechler at stat.math.ethz.ch> wrote:
> >>>>> 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() ?!
John Fox