Taras Zakharko
2021-Jun-30 11:23 UTC
[Rd] S3 dispatch does not work for generics defined inside an environment
I had another glance at the code and now I?m convinced that this is the bug in
registerS3method(). Default R behavior (in objects.c) appears to be to look for
method definitions in the top environment, not the defining environment, but
registerS3method() registers the method in the defining environment. I think
registerS3method() should be changed to something like:
- if (typeof(genfun) == "closure?)
- environment(genfun)
------------
+ if (typeof(genfun) == "closure?)
+ if(isFALSE(Sys.getenv("_R_S3_METHOD_LOOKUP_USE_TOPENV_AS_DEFENV_?))
environment(genfun) else topenv(environment(genfun))
in order to match the behavior of do_usemethod()
Best,
Taras
> On 30 Jun 2021, at 12:51, Taras Zakharko <taras.zakharko at uzh.ch>
wrote:
>
> @Duncan: .S3method() calls registerS3method() with appropriate
environmental argument under the good, so that?s not the problem.
>
> Anyway, I?ve been doing some debugging and I think I have found the issue.
The following snippet in src/objects.c
(https://github.com/wch/r-source/blob/ecc633b37d77fdd1cb27dda74d7f6b3684f0c01c/src/main/objects.c#L525)
sets the global lookup_use_topenv_as_defenv variable:
>
>
> if(lookup_use_topenv_as_defenv == -1) {
> lookup = getenv("_R_S3_METHOD_LOOKUP_USE_TOPENV_AS_DEFENV_");
> lookup_use_topenv_as_defenv =
> ((lookup != NULL) && StringFalse(lookup)) ? 0 : 1;
> }
>
> Isn?t that supposed to be
>
> lookup_use_topenv_as_defenv = ((lookup != NULL) &&
StringFalse(lookup)) ? 1 : 0;
>
> instead?
>
> The way the code works right now, methods will be looked up in top
environment exactly if _R_S3_METHOD_LOOKUP_USE_TOPENV_AS_DEFENV_ is not set.
This seems incompatible with what registerS3method() does (setting the
.__S3MethodsTable__. on the defining environment instead of the topenv). When I
change 0 and 1 around, everything works as expected.
>
> In the meantime, I can work around it by manually injecting
__S3MethodsTable__ into .GlobalEnv (which is my topenv here).
>
> I can open a bug report, but I would like to wait for some more comments.
>
> Best,
>
> Taras
>
>> On 30 Jun 2021, at 12:39, Joshua Ulrich <josh.m.ulrich at
gmail.com> wrote:
>>
>> On Wed, Jun 30, 2021 at 5:17 AM Duncan Murdoch <murdoch.duncan at
gmail.com> wrote:
>>>
>>> On 30/06/2021 5:22 a.m., Taras Zakharko wrote:
>>>> Dear all,
>>>>
>>>> I have a generic function and a bunch of methods defined in a
separate environment. Here is a reduced example:
>>>>
>>>> env <- local({
>>>> # define the generic function and the method
>>>> myfun <- function(x) UseMethod("myfun")
>>>> myfun.myclass <- function(x) print("called
myfun.myclass?)
>>>>
>>>> # register the method
>>>> .S3method("myfun", "myclass",
myfun.myclass)
>>>>
>>>> environment()
>>>> })
>>>>
>>>> Since the method has been registered, I hoped that invocation
like this would work:
>>>>
>>>> env$myfun(structure(0, class = "myclass?))
>>>>
>>>> However, this results in a ?no applicable method" error.
>>>>
>>>> It is my understanding that registerS3method (called by
.S3method) will install the method string in the .__S3MethodsTable__. table of
the environment where the generic function is defined, and this table is
subsequently used by usemethod() inside R, so I am puzzled that the dispatch
does not work. I checked and the .__S3MethodsTable__. of env is indeed setup
correctly. I also tried manually adding the method string to the global
.__S3MethodsTable__. inside .BaseNamespaceEnv to no effect.
>>>>
>>>> In fact, the only way to make it work is to define either myfun
or myfun.myclas in the global environment, which is something I would like to
avoid.
>>>>
>>>> Thank you in advance for any pointers!
>>>>
>>>
>>> registerS3method has an additional parameter "envir"
which I believe
>>> would end up set to env in your code. So this works:
>>>
>>>> eval(expression(myfun(structure(0, class =
"myclass"))), envir = env)
>>> [1] "called myfun.myclass"
>>>
>>> You could probably also call registerS3method with envir specified
>>> appropriately and get your original expression to work.
>>>
>> That doesn't seem to work on 4.1.0 for me. The code below worked
for
>> me in Oct-2020, though I'm not sure what version of R I was using
at
>> the time. I was slow to upgrade to 4.0, so it was probably the latest
>> 3.x version.
>>
>> env <- new.env()
>> local({
>> # define the generic function and the method
>> myfun <- function(x) { UseMethod("myfun", x) }
>>
>> # register the method
>> registerS3method("myfun", "myclass",
>> function(x) { print("called myfun.myclass") },
>> envir = env)
>> }, envir = env)
>> attach(env)
>> myfun(structure(0, class = "myclass"))
>>
>>
>>> Duncan Murdoch
>>>
>>> ______________________________________________
>>> R-devel at r-project.org mailing list
>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>>
>>
>> --
>> Joshua Ulrich | about.me/joshuaulrich
>> FOSS Trading | www.fosstrading.com
>
Duncan Murdoch
2021-Jun-30 11:29 UTC
[Rd] S3 dispatch does not work for generics defined inside an environment
On 30/06/2021 7:23 a.m., Taras Zakharko wrote:> I had another glance at the code and now I?m convinced that this is the bug in registerS3method(). Default R behavior (in objects.c) appears to be to look for method definitions in the top environment, not the defining environment, but registerS3method() registers the method in the defining environment. I think registerS3method() should be changed to something like:It registers wherever you ask it to. The default is the defining environment. .S3method is the one that always uses the defining environment, since it has no way to override the default. Duncan Murdoch> > > - if (typeof(genfun) == "closure?) > - environment(genfun) > ------------ > + if (typeof(genfun) == "closure?) > + if(isFALSE(Sys.getenv("_R_S3_METHOD_LOOKUP_USE_TOPENV_AS_DEFENV_?)) environment(genfun) else topenv(environment(genfun)) > > in order to match the behavior of do_usemethod() > > Best, > > Taras > > >> On 30 Jun 2021, at 12:51, Taras Zakharko <taras.zakharko at uzh.ch> wrote: >> >> @Duncan: .S3method() calls registerS3method() with appropriate environmental argument under the good, so that?s not the problem. >> >> Anyway, I?ve been doing some debugging and I think I have found the issue. The following snippet in src/objects.c (https://github.com/wch/r-source/blob/ecc633b37d77fdd1cb27dda74d7f6b3684f0c01c/src/main/objects.c#L525) sets the global lookup_use_topenv_as_defenv variable: >> >> >> if(lookup_use_topenv_as_defenv == -1) { >> lookup = getenv("_R_S3_METHOD_LOOKUP_USE_TOPENV_AS_DEFENV_"); >> lookup_use_topenv_as_defenv >> ((lookup != NULL) && StringFalse(lookup)) ? 0 : 1; >> } >> >> Isn?t that supposed to be >> >> lookup_use_topenv_as_defenv = ((lookup != NULL) && StringFalse(lookup)) ? 1 : 0; >> >> instead? >> >> The way the code works right now, methods will be looked up in top environment exactly if _R_S3_METHOD_LOOKUP_USE_TOPENV_AS_DEFENV_ is not set. This seems incompatible with what registerS3method() does (setting the .__S3MethodsTable__. on the defining environment instead of the topenv). When I change 0 and 1 around, everything works as expected. >> >> In the meantime, I can work around it by manually injecting __S3MethodsTable__ into .GlobalEnv (which is my topenv here). >> >> I can open a bug report, but I would like to wait for some more comments. >> >> Best, >> >> Taras >> >>> On 30 Jun 2021, at 12:39, Joshua Ulrich <josh.m.ulrich at gmail.com> wrote: >>> >>> On Wed, Jun 30, 2021 at 5:17 AM Duncan Murdoch <murdoch.duncan at gmail.com> wrote: >>>> >>>> On 30/06/2021 5:22 a.m., Taras Zakharko wrote: >>>>> Dear all, >>>>> >>>>> I have a generic function and a bunch of methods defined in a separate environment. Here is a reduced example: >>>>> >>>>> env <- local({ >>>>> # define the generic function and the method >>>>> myfun <- function(x) UseMethod("myfun") >>>>> myfun.myclass <- function(x) print("called myfun.myclass?) >>>>> >>>>> # register the method >>>>> .S3method("myfun", "myclass", myfun.myclass) >>>>> >>>>> environment() >>>>> }) >>>>> >>>>> Since the method has been registered, I hoped that invocation like this would work: >>>>> >>>>> env$myfun(structure(0, class = "myclass?)) >>>>> >>>>> However, this results in a ?no applicable method" error. >>>>> >>>>> It is my understanding that registerS3method (called by .S3method) will install the method string in the .__S3MethodsTable__. table of the environment where the generic function is defined, and this table is subsequently used by usemethod() inside R, so I am puzzled that the dispatch does not work. I checked and the .__S3MethodsTable__. of env is indeed setup correctly. I also tried manually adding the method string to the global .__S3MethodsTable__. inside .BaseNamespaceEnv to no effect. >>>>> >>>>> In fact, the only way to make it work is to define either myfun or myfun.myclas in the global environment, which is something I would like to avoid. >>>>> >>>>> Thank you in advance for any pointers! >>>>> >>>> >>>> registerS3method has an additional parameter "envir" which I believe >>>> would end up set to env in your code. So this works: >>>> >>>>> eval(expression(myfun(structure(0, class = "myclass"))), envir = env) >>>> [1] "called myfun.myclass" >>>> >>>> You could probably also call registerS3method with envir specified >>>> appropriately and get your original expression to work. >>>> >>> That doesn't seem to work on 4.1.0 for me. The code below worked for >>> me in Oct-2020, though I'm not sure what version of R I was using at >>> the time. I was slow to upgrade to 4.0, so it was probably the latest >>> 3.x version. >>> >>> env <- new.env() >>> local({ >>> # define the generic function and the method >>> myfun <- function(x) { UseMethod("myfun", x) } >>> >>> # register the method >>> registerS3method("myfun", "myclass", >>> function(x) { print("called myfun.myclass") }, >>> envir = env) >>> }, envir = env) >>> attach(env) >>> myfun(structure(0, class = "myclass")) >>> >>> >>>> Duncan Murdoch >>>> >>>> ______________________________________________ >>>> R-devel at r-project.org mailing list >>>> https://stat.ethz.ch/mailman/listinfo/r-devel >>> >>> >>> >>> -- >>> Joshua Ulrich | about.me/joshuaulrich >>> FOSS Trading | www.fosstrading.com >> > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel >