Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

visibility issue with Rcpp module classes and devtools::load_all #222

Closed
jlmelville opened this issue Aug 2, 2022 · 3 comments
Closed

Comments

@jlmelville
Copy link

(Apologies if this is a duplicate: I have searched for issues that combine Rcpp modules with devtools/pkgload but did not find anything from recent years.)

I've noticed some behavior with Rcpp modules and package loading with devtools::load_all where the expected class name does not seem to be visible in the namespace in ways I would expect, i.e. if we export a class in the NAMESPACE then <pkg_name>::<class_name> should exist.

I am able to trigger the issue with the rcpp-modules-student example, but I have also found it when working with the source code of CRAN packages such as RcppHNSW and RcppAnnoy which also use Rcpp modules to expose C++ classes.

To be concrete, in the rcpp-modules-student example, where RcppStudent is the package name and Student is the exported class, I would expect RcppStudent::Student to exist.

When loading the package via devtools::load_all(".") the name of the class doesn't seem to be present in the namespace the first time, load_all is called. Here's a sample output (based on opening an R terminal in a git checkout of rcpp-modules-student), where after the first load_all, RcppStudent::Student does not exist, but after a second load_all, it does:

> devtools::load_all(".")
ℹ Loading RcppStudent
Warning message:
Objects listed as exports, but not present in namespace:Student 
> RcppStudent::Student
Error: 'Student' is not an exported object from 'namespace:RcppStudent'
> devtools::load_all(".")
ℹ Loading RcppStudent
> RcppStudent::Student
C++ class 'Student' <0000017fa1d830d0>
Constructors:
    Student(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int, bool)
    Student(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int, bool)

Fields: No public fields exposed by this class

Methods: 
     int GetAge()  
           
     int GetAge()  
           
     std::vector<int, std::allocator<int> > GetFavoriteNumbers()  
           
     std::vector<int, std::allocator<int> > GetFavoriteNumbers()  
           
     std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > GetName()  
           
     std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > GetName()  
           
     bool IsMale()  
           
     bool IsMale()  
           
     bool LikesBlue()  
           
     bool LikesBlue()  

If I install the package via devtools::install_local, by e.g. opening an R shell in a sibling folder and running devtools::install_local("../rcpp-modules-student") then RcppStudent::Student is available straight away.

I don't claim to understand how the name lookup works in R, but if I ignore the warning that Student isn't exported and try to create a new instance with new(Student) after load_all that works:

# check there is no Student before loading:
> str(new(Student))
Error in .getClassesFromCache(Class) : object 'Student' not found

# load the first time
> devtools::load_all(".")
ℹ Loading RcppStudent
Warning message:
Objects listed as exports, but not present in namespace:Student 

# create a Student anyway
> str(new(Student))
Reference class 'Rcpp_Student' [package "RcppStudent"] with 0 fields
 list()
 and 21 methods, of which 7 are  possibly relevant:
   finalize, GetAge, GetFavoriteNumbers, GetName, initialize, IsMale, LikesBlue

...but Student is still not in the RcppStudent namespace.

> RcppStudent::Student
Error: 'Student' is not an exported object from 'namespace:RcppStudent'
Error during wrapup: Expecting an external pointer: [type=environment].
Error: no more error handlers available (recursive errors?); invoking 'abort' restart

I am not sure if those errors are relevant (I ran this in RStudio). I see this on Windows as well as Linux. I don't know when this started, but I just tried it with R 3.6, 4.0, 4.1 and 4.2 and they all show the same behavior. I am sure I had been happily using devtools::load_all with Rcpp modules on at least R 3.6, but I may not have tried it with R 4.x until recently. So maybe there has been a change in how pkgload and Rcpp interact with more recent versions of those libraries? Also, this doesn't affect C++ functions, only when wrapping classes.

Other things:

  • new(Student) works but new("Student") does not.
  • from running str(new(Student)) the output says Reference class 'Rcpp_Student'.
  • calling new("Rcpp_Student") does work.
  • but there is no RcppStudent::Rcpp_Student.
  • and there is nothing in the rcpp-modules-student project that uses the identifier Rcpp_Student.
  • so is there some confusion occurring between the name of the C++ class Student and the reference class Rcpp_Student?

Here is a sample sessionInfo for when I am using devtools::load_all("."):

sessionInfo()
R version 4.2.1 (2022-06-23 ucrt)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 22000)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.utf8  LC_CTYPE=English_United States.utf8   
[3] LC_MONETARY=English_United States.utf8 LC_NUMERIC=C                          
[5] LC_TIME=English_United States.utf8    

attached base packages:
[1] stats     graphics  grDevices datasets  utils     methods   base     

other attached packages:
[1] RcppStudent_0.0.1 testthat_3.1.4   

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.9        compiler_4.2.1    later_1.3.0       urlchecker_1.0.1  prettyunits_1.1.1 profvis_0.3.7    
 [7] remotes_2.4.2     tools_4.2.1       digest_0.6.29     pkgbuild_1.3.1    pkgload_1.3.0     memoise_2.0.1    
[13] lifecycle_1.0.1   rlang_1.0.4       shiny_1.7.2       cli_3.3.0         rstudioapi_0.13   fastmap_1.1.0    
[19] withr_2.5.0       stringr_1.4.0     desc_1.4.1        fs_1.5.2          htmlwidgets_1.5.4 devtools_2.4.4   
[25] rprojroot_2.0.3   glue_1.6.2        R6_2.5.1          processx_3.7.0    sessioninfo_1.2.2 purrr_0.3.4      
[31] callr_3.7.1       magrittr_2.0.3    codetools_0.2-18  usethis_2.1.6     promises_1.2.0.1  ps_1.7.1         
[37] ellipsis_0.3.2    htmltools_0.5.3   mime_0.12         xtable_1.8-4      renv_0.15.5       httpuv_1.6.5     
[43] stringi_1.7.8     miniUI_0.1.1.1    cachem_1.0.6      crayon_1.5.1      brio_1.1.3       

For the sibling folder where devtools::install_local works:

sessionInfo()
R version 4.2.1 (2022-06-23 ucrt)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 22000)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.utf8  LC_CTYPE=English_United States.utf8   
[3] LC_MONETARY=English_United States.utf8 LC_NUMERIC=C                          
[5] LC_TIME=English_United States.utf8    

attached base packages:
[1] stats     graphics  grDevices datasets  utils     methods   base     

loaded via a namespace (and not attached):
[1] compiler_4.2.1    tools_4.2.1       Rcpp_1.0.9        RcppStudent_0.0.1 codetools_0.2-18 
[6] renv_0.15.5  
@lionel-
Copy link
Member

lionel- commented Sep 20, 2023

Thanks for the detailed write-up. Better interaction with Rcpp module classes is not on the short or medium term road map. If you want to take a stab at improving support in pkgload, I'd be happy to review a PR though!

Looks like the relevant parts to figure out where the instantiation fails are in https://github.com/RcppCore/Rcpp/blob/master/R/loadModule.R (but I might be wrong).

@jlmelville
Copy link
Author

Thanks for taking a look @lionel. I think it's safe to assume that this isn't a problem that is affecting many people, so I will close this.

FWIW my workaround was to not use roxygen2 and its @export feature to generate the NAMESPACE. Instead, I manually export classes and functions with exportPattern. For my package, the functions I want to export begin with hnsw_ and the classes with Hnsw, so this was fairly straightforward and low-risk:

exportPattern("^[Hh]nsw")

devtools::load_all was happy.

@lionel-
Copy link
Member

lionel- commented Sep 21, 2023

Thanks for sharing your workaround!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants