Re: [R-pkg-devel] failing S3 dispatch
On 21/10/2021 1:23 p.m., Jens Oehlschlägel wrote: Thanks Duncan, I finally found the reason for the mysterious dispatch-failure: I had an unwanted and unexported replicated definition of the 'clone' generic in ff's namespace (a left-over). I still don't understand how this prevented the proper dispatch since the duplicate in ff's namespace it was *not* called. I further experimented and *any* non-exported object matching the name of the generic caused the problem. Scary, maybe worth a check! Your NAMESPACE file contains S3method("clone",ff) When R installs your package, it has to figure out where "clone" lives. You imported a copy of it from bit, and then overwrote that imported copy with the replicated definition. That created a new generic belonging to ff, and that's the one that your clone.ff method was attached to. When you called clone(x), you called the bit::clone generic which never received the registration, so dispatch to clone.ff never happened. It makes sense: you don't want a generic in one package to interfere with an unrelated generic in another package that happens to have the same name. Duncan Murdoch Anyhow, removing the non-exported object solved the problem. Best regards Jens On 20.10.21 13:43, Jens Oehlschlägel wrote: Thank you Duncan, bit NAMESPACE has S3method(clone,default) export(clone) ff NAMESPACE has import(bit) # wish of CRAN maintainers: export another time here (now maintained and exported in bit) # without this R CMD CHECK complained, but with it R CMD CHECK complains also, how to export again and why? # clone #,clone.default clone.ff ,clone.ffdf S3method("clone",ff) S3method(clone, ffdf) # wish of CRAN maintainers: export another time here (now maintained and exported in bit) #S3method(clone, default) Best Jens On 20.10.21 11:02, Duncan Murdoch wrote: On 19/10/2021 3:43 p.m., Jens Oehlschlägel wrote: I didn't find an answer elsewhere: My package 'bit' creates a S3 generic 'clone' and exports it. Furthermore it registers a S3 method 'clone.default' (not exported). My package 'ff' imports package 'bit' and exports and registers a new S3 method 'clone.ff'. However, calling 'clone(ffobj)' dispatches to clone.default instead of clone.ff !? Why? You should show us the NAMESPACE entries involving clone and clone.ff from ff. Some comments that may or may not be relevant: - Normally you wouldn't export clone.ff, it's enough to register it using S3method(). - You may have created a new generic named clone, and that's what clone.ff would attach itself to. You can have bit::clone and ff::clone as different generics and that would cause problems. What is the recommended way to create new S3-methods that get dispatched? In earlier versions of the packages I simply exported everything - that worked. I import the generic and use S3method(generic, method). I don't export the methods, so I wouldn't be able to call z <- clone.ff(a). Duncan Murdoch Best Jens > require(ff) > > a <- as.ff(0:9) > class(x) [1] "ff_vector" "ff" > > x <- clone(a) > y <- bit:::clone.default(a) > z <- clone.ff(a) > > # cloned ffobjects should have different filenames> > filename(a) # original [1] "/tmp/Rtmpk17JRZ/ff/clone1ed54cbb5060.ff" > > filename(x) # unexpected equal (dispatch to clone.default) [1] "/tmp/Rtmpk17JRZ/ff/clone1ed54cbb5060.ff" > > filename(y) # expected equal [1] "/tmp/Rtmpk17JRZ/ff/clone1ed54cbb5060.ff" > > filename(z) # OK [1] "/tmp/Rtmpk17JRZ/ff/clone1ed551d3ee66.ff" > version _ platform x86_64-pc-linux-gnu arch x86_64 os linux-gnu system x86_64, linux-gnu status major 4 minor 1.1 year 2021 month 08 day 10 svn rev 80725 language R version.string R version 4.1.1 (2021-08-10) nickname Kick Things __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel
Re: [R-pkg-devel] failing S3 dispatch
Thanks Duncan, I finally found the reason for the mysterious dispatch-failure: I had an unwanted and unexported replicated definition of the 'clone' generic in ff's namespace (a left-over). I still don't understand how this prevented the proper dispatch since the duplicate in ff's namespace it was *not* called. I further experimented and *any* non-exported object matching the name of the generic caused the problem. Scary, maybe worth a check! Anyhow, removing the non-exported object solved the problem. Best regards Jens On 20.10.21 13:43, Jens Oehlschlägel wrote: Thank you Duncan, bit NAMESPACE has S3method(clone,default) export(clone) ff NAMESPACE has import(bit) # wish of CRAN maintainers: export another time here (now maintained and exported in bit) # without this R CMD CHECK complained, but with it R CMD CHECK complains also, how to export again and why? # clone #,clone.default clone.ff ,clone.ffdf S3method("clone",ff) S3method(clone, ffdf) # wish of CRAN maintainers: export another time here (now maintained and exported in bit) #S3method(clone, default) Best Jens On 20.10.21 11:02, Duncan Murdoch wrote: On 19/10/2021 3:43 p.m., Jens Oehlschlägel wrote: I didn't find an answer elsewhere: My package 'bit' creates a S3 generic 'clone' and exports it. Furthermore it registers a S3 method 'clone.default' (not exported). My package 'ff' imports package 'bit' and exports and registers a new S3 method 'clone.ff'. However, calling 'clone(ffobj)' dispatches to clone.default instead of clone.ff !? Why? You should show us the NAMESPACE entries involving clone and clone.ff from ff. Some comments that may or may not be relevant: - Normally you wouldn't export clone.ff, it's enough to register it using S3method(). - You may have created a new generic named clone, and that's what clone.ff would attach itself to. You can have bit::clone and ff::clone as different generics and that would cause problems. What is the recommended way to create new S3-methods that get dispatched? In earlier versions of the packages I simply exported everything - that worked. I import the generic and use S3method(generic, method). I don't export the methods, so I wouldn't be able to call z <- clone.ff(a). Duncan Murdoch Best Jens > require(ff) > > a <- as.ff(0:9) > class(x) [1] "ff_vector" "ff" > > x <- clone(a) > y <- bit:::clone.default(a) > z <- clone.ff(a) > > # cloned ffobjects should have different filenames> > filename(a) # original [1] "/tmp/Rtmpk17JRZ/ff/clone1ed54cbb5060.ff" > > filename(x) # unexpected equal (dispatch to clone.default) [1] "/tmp/Rtmpk17JRZ/ff/clone1ed54cbb5060.ff" > > filename(y) # expected equal [1] "/tmp/Rtmpk17JRZ/ff/clone1ed54cbb5060.ff" > > filename(z) # OK [1] "/tmp/Rtmpk17JRZ/ff/clone1ed551d3ee66.ff" > version _ platform x86_64-pc-linux-gnu arch x86_64 os linux-gnu system x86_64, linux-gnu status major 4 minor 1.1 year 2021 month 08 day 10 svn rev 80725 language R version.string R version 4.1.1 (2021-08-10) nickname Kick Things __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel
Re: [R-pkg-devel] Is there a better way ...?
Duncan's version is much clearer than my solution, and the only reason I use my version is so that the source reference of the function looks neater, and so that auto-code-indentation won't mess up my source reference either. If none of that made sense, don't worry about it, use Duncan's approach. I think the advantage of Duncan's solution is that getting and setting .fooInfo is more compact. It's not clear exactly how you're modifying .fooInfo, but I'll assume plot.foo is modifying it. Using Duncan's approach, you might do something like: plot.foo <- local({ .fooInfo <- 0 function (...) { value <- .fooInfo # get .fooInfo .fooInfo <<- .fooInfo + 1 # set .fooInfo, you must use <<- here instead of <- return(value) } }) and Deepayan's approach: ..foo.env <- local({ .fooInfo <- 0 environment() }) plot.foo <- function (...) { value <- .foo.env$.fooInfo # get .fooInfo .foo.env$.fooInfo <- .foo.env$.fooInfo + 1 # set .fooInfo return(value) } Both of these work perfectly fine, so you don't have to worry too much about which you implement. The differences are mostly just visual appearance, they have nearly equivalent functionality and performance. On Thu, Oct 21, 2021 at 2:45 AM Rolf Turner wrote: > > On Thu, 21 Oct 2021 02:03:41 -0400 > Duncan Murdoch wrote: > > > On 21/10/2021 12:40 a.m., Andrew Simmons wrote: > > > I think the simplest answer is to store the variable in the > > > functions frame. I'm assuming here that the only plot.foo needs > > > access to .fooInfo, if not this can be changed. > > > > > > > > > plot.foo <- function (...) > > > { > > > .fooInfo > > > } > > > environment(plot.foo) <- new.env() > > > evalq({ > > > .fooInfo <- NULL > > > }, environment(plot.foo)) > > > > > > > > > Make your function, and do whatever you need with .fooInfo within > > > said function. Whenever you previously updated .fooInfo in the > > > global environment, update .fooInfo in plot.foo environment instead. > > > Also, because .fooInfo is not stored in the package's frame, it > > > won't be locked when the namespace is sealed. If you created it at > > > the toplevel, that would create some issues. But this works fine. > > > > I agree with the final result, but I'd write the code differently: > > > > plot.foo <- local({ > > > >.fooInfo <- NULL > > > >function (...) { ... } > > }) > > > > creates an environment, puts .fooInfo into it with value NULL, then > > creates a function with that environment attached and returns it. > > > > I think Andrew's approach will work, but changing a function's > > environment always worries me. Using local(), the function assigned > > to plot.foo never has a different environment than the one it ends up > > with (and I don't need to remember how evalq() works). > > Thanks everyone for these suggestions. They seem a great deal > less shaganappi/kludgy than my previous approaches. > > I've never really felt totally comfortable with the environment > concept, despite have used it quite a bit (basically in a > hammer-and-hope style.) > > Can anyone comment on the difference between Deepayan's suggestion > (create a new environment in the package) and Duncan's suggestion > (create an environment that is local to plot.foo())? Are there pros > and cons between the two? > > And Deepayan: what is the rationale for not exporting the new > environment that you suggest creating? Presumably this guards against > something. What? I'd just like to extend my (currently minimal) > comprehension of the issues. > > I must admit that Andrew's suggestion kind of overwhelms and bewilders > me. I really have no idea what evalq() does. I guess I could RTFM, > but the thought of doing that scares me! :-) > > Thanks again everybody. > > cheers, > > Rolf > > -- > Honorary Research Fellow > Department of Statistics > University of Auckland > Phone: +64-9-373-7599 ext. 88276 > > [[alternative HTML version deleted]] __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel
Re: [R-pkg-devel] Is there a better way ...?
> Duncan Murdoch > on Thu, 21 Oct 2021 08:09:02 -0400 writes: > I agree with almost everything Deepayan said, but would add one thing: > On 21/10/2021 3:41 a.m., Deepayan Sarkar wrote: > ... >> My suggestion is having a package-specific environment, and Duncan's >> is to have a function-specific environment. If you only need this for >> this one function, then that should be good enough. If you eventually >> want to access the persistent information from multiple functions, >> having a package-specific environment would be more useful. > I agree with that statement, but those aren't the only two choices. > Your local() call can create several functions and return them in a > list; then just those functions have access to the local variables. For > example, > createFns <- local({ > .fooInfo <- NULL > fn1 <- function (...) { ... } > fn2 <- function (...) { ... } > list(fn1 = fn1, fn2 = fn2) > }) > fns <- createFns() > fn1 <- fns$fn1 > fn2 <- fns$fn2 > Now fn1 and fn2 are functions that can see .fooInfo, and nobody else can > (without going through contortions). > One other difference between this approach and the package-specific > environment: there's only one package-specific environment in > Deepayan's formulation, but I could call createFns() several times, > creating several pairs of functions, each pair with its own independent > version of .fooInfo. > I don't know if that's something that would be useful to you, but > conceivably you'd want to maintain partial plots in several different > windows, and that would allow you to do so. Note that the above approach has been how nls() has been implemented for R ... a very long time ago {before R 1.0.0} e.g. from example(nls) : DNase1 <- subset(DNase, Run == 1) fm1 <- nls(density ~ SSlogis(log(conc), Asym, xmid, scal), DNase1) str(fm1 $ m) > List of 16 > $ resid :function () > $ fitted:function () > $ formula :function () > $ deviance :function () > $ lhs :function () > $ gradient :function () > $ conv :function () > $ incr :function () > $ setVarying:function (vary = rep_len(TRUE, np)) > $ setPars :function (newPars) > $ getPars :function () > $ getAllPars:function () > $ getEnv:function () > $ trace :function () > $ Rmat :function () > $ predict :function (newdata = list(), qr = FALSE) > - attr(*, "class")= chr "nlsModel" ## so 16 functions, all sharing the *same* environment very ## efficiently and nicely ## this is *the* environment for the fitted model : fmE <- environment(fm1$m[[1]]) ls.str(fmE) > convCrit : function () > dev : num 0.00479 > env : > form : Class 'formula' language density ~ SSlogis(log(conc), Asym, xmid, > scal) > getPars : function () > > > so the environment "contains" the functions themselves (but quite a few more things) and for an environment that means it only has pointers to the same function objects which are *also* in `fm1$m`. So, there has been a nice convincing and important example on how to do this - inside R for more than two decennia. Martin Maechler __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel
Re: [R-pkg-devel] Is there a better way ...?
I agree with almost everything Deepayan said, but would add one thing: On 21/10/2021 3:41 a.m., Deepayan Sarkar wrote: ... My suggestion is having a package-specific environment, and Duncan's is to have a function-specific environment. If you only need this for this one function, then that should be good enough. If you eventually want to access the persistent information from multiple functions, having a package-specific environment would be more useful. I agree with that statement, but those aren't the only two choices. Your local() call can create several functions and return them in a list; then just those functions have access to the local variables. For example, createFns <- local({ .fooInfo <- NULL fn1 <- function (...) { ... } fn2 <- function (...) { ... } list(fn1 = fn1, fn2 = fn2) }) fns <- createFns() fn1 <- fns$fn1 fn2 <- fns$fn2 Now fn1 and fn2 are functions that can see .fooInfo, and nobody else can (without going through contortions). One other difference between this approach and the package-specific environment: there's only one package-specific environment in Deepayan's formulation, but I could call createFns() several times, creating several pairs of functions, each pair with its own independent version of .fooInfo. I don't know if that's something that would be useful to you, but conceivably you'd want to maintain partial plots in several different windows, and that would allow you to do so. And there are other choices too: there are several packages implementing object systems that allow objects to maintain persistent data. I haven't used those, so this list may contain omissions and errors: R6, R.oo, proto. Duncan Murdoch __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel
Re: [R-pkg-devel] Is there a better way ...?
On Thu, Oct 21, 2021 at 12:15 PM Rolf Turner wrote: > > > On Thu, 21 Oct 2021 02:03:41 -0400 > Duncan Murdoch wrote: > > > On 21/10/2021 12:40 a.m., Andrew Simmons wrote: > > > I think the simplest answer is to store the variable in the > > > functions frame. I'm assuming here that the only plot.foo needs > > > access to .fooInfo, if not this can be changed. > > > > > > > > > plot.foo <- function (...) > > > { > > > .fooInfo > > > } > > > environment(plot.foo) <- new.env() > > > evalq({ > > > .fooInfo <- NULL > > > }, environment(plot.foo)) > > > > > > > > > Make your function, and do whatever you need with .fooInfo within > > > said function. Whenever you previously updated .fooInfo in the > > > global environment, update .fooInfo in plot.foo environment instead. > > > Also, because .fooInfo is not stored in the package's frame, it > > > won't be locked when the namespace is sealed. If you created it at > > > the toplevel, that would create some issues. But this works fine. > > > > I agree with the final result, but I'd write the code differently: > > > > plot.foo <- local({ > > > >.fooInfo <- NULL > > > >function (...) { ... } > > }) > > > > creates an environment, puts .fooInfo into it with value NULL, then > > creates a function with that environment attached and returns it. > > > > I think Andrew's approach will work, but changing a function's > > environment always worries me. Using local(), the function assigned > > to plot.foo never has a different environment than the one it ends up > > with (and I don't need to remember how evalq() works). > > Thanks everyone for these suggestions. They seem a great deal > less shaganappi/kludgy than my previous approaches. > > I've never really felt totally comfortable with the environment > concept, despite have used it quite a bit (basically in a > hammer-and-hope style.) > > Can anyone comment on the difference between Deepayan's suggestion > (create a new environment in the package) and Duncan's suggestion > (create an environment that is local to plot.foo())? Are there pros > and cons between the two? My suggestion is having a package-specific environment, and Duncan's is to have a function-specific environment. If you only need this for this one function, then that should be good enough. If you eventually want to access the persistent information from multiple functions, having a package-specific environment would be more useful. I'm not sure what you are trying to do, but I can't see how you can do something sensible with a function-specific environment if someone does plot.foo(something) plot.default(1:10) plot.foo(something else, add = TRUE) So maybe you would eventually want to set a hook (?setHook) for plot.new to ensure that no other plot has been created in between, which could write into this package-specific environment. > And Deepayan: what is the rationale for not exporting the new > environment that you suggest creating? Presumably this guards against > something. What? I'd just like to extend my (currently minimal) > comprehension of the issues. Nothing other than the usual reason for not exporting things unnecessarily, which is to not pollute the user workspace. > I must admit that Andrew's suggestion kind of overwhelms and bewilders > me. I really have no idea what evalq() does. I guess I could RTFM, > but the thought of doing that scares me! :-) Andrew's suggestion looks more complicated than it is. Think of .fooInfo as a "global" variable, just in your package namespace rather than .GlobalEnv, so you could do (in your package code) .fooInfo <- NULL plot.foo <- function(...) { if (is.null(.fooInfo)) ... # use .fooInfo .fooInfo <<- something # set .fooInfo } Andrew suggested a separate (and unnamed) environment to store both .fooInfo and plot.foo, so the setting part becomes a bit more complicated (but accessing becomes safer in the sense that no other function can access .fooInfo). My suggestion is essentially similar, except that you can use <- instead of <<- because it's an environment. .fooEnv <- new.env() plot.foo <- function(...) { if (is.null(.fooEnv$info)) ... # use .fooEnv$info .fooEnv$info <- something # set .fooEnv$info } Best, -Deepayan > Thanks again everybody. > > cheers, > > Rolf > > -- > Honorary Research Fellow > Department of Statistics > University of Auckland > Phone: +64-9-373-7599 ext. 88276 > __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel