[Rd] RFC: Declaring "foo.bar" as nonS3method() ?!
Martin Maechler
maechler at stat.math.ethz.ch
Sat Jun 13 13:35:41 CEST 2015
>>>>> 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
More information about the R-devel
mailing list