Rcpp: packages that use modules with classes don't work unless Rcpp is attached

Created on 18 Aug 2014  Â·  39Comments  Â·  Source: RcppCore/Rcpp

Packages that use Rcpp modules which expose C++ classes work only if Rcpp is explicitly present on the search path. This has been described, but mis-identified in #118.

To replicate, create a skeleton package via Rcpp::Rcpp.package.skeleton(module=TRUE), build, install and run:

> library(anRpackage)
Warning message:
 class "C++Object" is defined (with package slot ‘Rcpp’) but no metadata object found to revise subclass information---not exported?  Making a copy in package ‘anRpackage’ 
> World$new()
Error in .Object$initialize(...) : 
  could not find function "cpp_object_initializer"

Note that the skeleton uses Imports: Rcpp which is crucial here. If Rcpp is explicitly attached, it works:

> library(Rcpp)
> library(anRpackage)
> World$new()
C++ object <0x1136d40> of class 'World' <0xa6c8b0>

This turns out to be a major problem for the following reason: A package can try to work around this problem by using Depends: Rcpp, but that only applies if the package is also attached itself. This means that the "I don't work if not attached" property becomes contagious in that it bubbles through the package dependency chain - if a package has ever even one ancestor in the dependency chain that is a package with Rcpp module then it itself becomes unusable unless Rcpp attached - so in theory all packages must depend on Rcpp - whether they use it or not. Even if desirable ;), it's not practical, unfortunately.

documentation

Most helpful comment

3.1.1 of the modules vignette already recommends "import(Rcpp)" for the NAMESPACE of packages that use Rcpp modules.

All 39 comments

Thanks for the detailed analysis. But I am wondering how this fools R CMD check for packages that use Imports: and not Depends:. If I understand you correctly these should now work. Yet they do (eg RcppCNPy). Any idea?

I guess the regression test does not use module=TRUE :-/

It doesn't if the package actually tests module calls. In the template example replace

\examples{
\dontrun{
rcpp_hello_world()
}
}

with

\examples{
rcpp_hello_world()
World$new()
}

and you'll get

* checking examples ... ERROR
Running examples in ‘anRpackage-Ex.R’ failed
The error most likely occurred in:

> ### Name: rcpp_hello_world
> ### Title: Simple function using Rcpp
> ### Aliases: rcpp_hello_world
> 
> ### ** Examples
> 
> rcpp_hello_world()
[[1]]
[1] "foo" "bar"

[[2]]
[1] 0 1

> World$new()
Error in .Object$initialize(...) : 
  could not find function "cpp_object_initializer"
Calls: <Anonymous> ... <Anonymous> -> initialize -> initialize -> <Anonymous>
Execution halted

As for RcppCNPy, AFAIK it doesn't wrap a class, the module only exports functions - that may be the triggering point. I'm not an Rcpp guru, so I don't know enough about the internals, but the C++Object wrapper class may have something to do with it - i.e. the issue may exist only when a C++ class is reflected to R in the module.

If we add one more line to NAMESPACE, and (as usual) load the methods package, things are better:

edd@max:/tmp$ Rscript -e 'print(search()); library(methods); library(anRpackage); World$new()'
[1] ".GlobalEnv"        "package:stats"     "package:graphics"  "package:grDevices" "package:utils"     "package:datasets"  "Autoloads"
[8] "package:base"
Warning message:
class "C++Object" is defined (with package slot ‘Rcpp’) but no metadata object found to revise subclass information---not exported?  Making a copy in package ‘anRpackage’
C++ object <0x1da3500> of class 'World' <0x210e530>
edd@max:/tmp$ 

Given that there was an explicit warning, I just added this one:

edd@max:/tmp$ tail -1 anRpackage/NAMESPACE 
importFrom(Rcpp, evalCpp, cpp_object_initializer)
edd@max:/tmp$ 

Ok, thanks, importing cpp_object_initializer (explicitly or implicitly) makes this go away for that package.

Unfortunately, I suspect there is something deeper involved - when there is one extra level of dependency, the error returns (that is the case for my original problem that triggered me to look into this where I have gitgist imports guitar imports Rcpp and class calls triggered from gitgists will fail). I suspect that it has to do with this warning:

Warning message:
class "C++Object" is defined (with package slot ‘Rcpp’) but no metadata object found to revise subclass information---not exported?  Making a copy in package ‘.GlobalEnv’ 

because it means the scope of the class may the wrong and it will be only looking the the search path instead of using the package as it should. I'll try to cook up a minimal example...

Fair enough re RcppCNPy, but RcppBDT is a better, more complete example with classes.

The whole issue may be related to the 'no linking' init step, and rather hard/impossible to debug.

I had a peek at the init and I think it's possible to trace, but will take substantial effort to do it - this may be related to the delayed module registration or some such. I don't have enough time to dig deeper right now, but I need to get this fixed before our next RCloud release so I'll put it on my ToDo list and keep you posted.

That would be very, very welcome. Much appreciated.

I can confirm that this is essentially a scoping issue. As far as I can tell (and this is to be taken with some grain of salt since the Rcpp module code contains a lot of voodoo that I cannot claim to fully understand) the problem is how modules are registered - since they are populated in the package's namespace by hand, the S4 class metadata is missing in the package's namespace (see ls(asNamespace(..), all=TRUE) - . This is what the warning above is about - normally, the class metadata would be updated in the package's namespace, but since it's missing S4 falls back to updating in the the workspace (see ls(all=TRUE)) - a bad idea here. This means that instead of package namespace, the global environment is used for the S4 metadata - which only works if everything is attached since this bypasses the regular namespace chain. This is what you get for using S4 ;). The proper fix would be to make sure the metadata is populated in the package namespace when loading the modules.

Thanks for the follow-up and analysis. Modules code is indeed close to voodoo. So maybe this is an issue to be documented ("you currently need to attach Rcpp when using modules with classes") and to be tabled for a potential future improvement?

Well, the problem with "you currently need to attach Rcpp when using modules with classes" is that it is transitive, so it's more like "if you ever import an Rcpp package with modules then you and anyone using your package has to attach Rcpp" - which is not realistic. Note that the import issue is just a symptom of the deeper issue of defining classes in the wrong place - so I'd still maintain this is a bug that needs fixing. The issue is finding whoever knows the module registration voodoo so it can be fixed :)

FWIW, I resolved the warning in my own package by making documentation for each of the S4 classes associated with each Rcpp module. A bit of a hack, but it removes the need to attach Rcpp.

Ok, For reference, do you have a link or example?

This issue can be mitigated by adding the following to NAMESPACE of a package:

import(Rcpp)

While this does not deal with the issue, it resolved the symptom.
Note: this is needed even if importFrom(Rcpp, evalCpp) is present.

We have always recommend the following form:

importFrom(Rcpp, evalCpp)

and of course other people did variants around that for no good reason. _Any_ symbol from Rcpp, once forced to instantiate, should do. A plain import(Rcpp) sometimes works, what we have here works more often and is not really anymore complicated.

Yes, I have taken the snippet from Rcpp-modules vignette. I stumbled on this by googling the error. Figured it would be a good place to leave a note.

Maybe that is a good item to add to the Rcpp-FAQ too.

I misunderstood what importFrom() actually meant. Updated comment above.

If we knew how/why/where that is needed.... I have packages with modules, and I only do importFrom(Rcpp, evalCpp). I have yet to find it insufficient. Can you produce a minimal reproducible example that _does_ need _both_ lines?

(And I now updated my comment above too)

Here is a small example where I see the issue: https://github.com/ivan-krukov/mre168

Seems related to using modules to instantiate C++ class objects?

Really good minimally reproducible example, thanks for setting it up.

And I concur: it seems one needs to import Rcpp.

I have one counterexample in RcppXts -- but you could argue that its use of Modules is lighter-weight and does not instantiate classes (just how @ha0ye surmised).

Some debugging information about the environment that initialize uses. In the working case:

Browse[6]> pryr::parenvs(environment())
  label                           name            
1 <environment: 0x103657db8>      ""              
2 <environment: 0x106cac678>      ""              
3 <environment: namespace:mre168> ""              
4 <environment: 0x101d51910>      "imports:mre168"
5 <environment: namespace:base>   ""              
6 <environment: R_GlobalEnv>      ""     

Browse[6]> pryr::where("cpp_object_initializer")
<environment: 0x101d51910>
attr(,"name")
[1] "imports:mre168"

In the 'not working' case, it looks like the mre168 namespace doesn't end up importing the required symbol cpp_object_initializer.

tl;dr: packages that want to use modules _must_ import the entire Rcpp namespace, since the functions invoked will depend on objects on that namespace (and so we need to make sure that they're imported by the client package).

Or, we have to 'audit' modules and figure out what functions from the Rcpp namespace will be required by client packages, and ask package users to specifically import those as well.

Yeppers. We probably need to straighten the documentation out. Strictly speaking it is _most_ packages using Modules (see my RcppXts counterexample), but we may as well recommended it outright.

@eddelbuettel: Doc flag please + 0.12.9 label?

Whoops, looks like I can set milestones but not add new ones.

As an aside, we might also want to update the behavior of Rcpp.package.skeleton() to ensure the NAMESPACE file is populated as such when modules are used.

We have this in Rcpp.package.skeleton

    if (isTRUE(module)) {
        writeLines('import(methods, Rcpp)', ns)
        message(" >> added import(methods, Rcpp) directive to NAMESPACE")
    } else {
        writeLines('importFrom(Rcpp, evalCpp)', ns)
        message(" >> added importFrom(Rcpp, evalCpp) directive to NAMESPACE" )
    }

and the import(methods, Rcpp) is actually what most of my packages using Modules. Maybe no need for change after all. We _already_ special-case Modules.

3.1.1 of the modules vignette already recommends "import(Rcpp)" for the NAMESPACE of packages that use Rcpp modules.

Well then! Perhaps the only thing that may be warranted here is an FAQ entry? Or even no action at all?

... saying "just trust Rcpp.package.skeleton(pkgName, module=TRUE)" ?

Don't know. Maybe we are actually done. Code seems correct, documentation seems correct.

I followed the vignette and didn't have a problem. At one point I tried "importFrom(Rcpp, evalCpp)", found it didn't work, and reverted. Curious to know how @ivan-krukov stumbled onto the issue to see if there's a gap somewhere that should be patched.

@ha0ye My example was mostly taken from the auto-generated RStudio "Package with Rcpp" template. This of course, has nothing to do with modules. So I suppose the issue could be closed.

Yes, that is more between you and RStudio for thinking it would just work with Modules. But it needs subtle adjustments.

I'm in favor of closing this.

+1 (I'll double check to see if we need to change anything on the RStudio side)

If you have 'infinite capacity' another toggle for 'Package w/Rcpp' _and modules_ could help.
But there are too many options as it stands.

Closing this now though. This seems known, documented and supported in the code (even though some of us, yours truly included, occasionally give less perfect recommendations).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

nathan-russell picture nathan-russell  Â·  3Comments

steve-s picture steve-s  Â·  4Comments

jgellar picture jgellar  Â·  8Comments

kevinushey picture kevinushey  Â·  6Comments

iago-pssjd picture iago-pssjd  Â·  4Comments