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

Extending Rcpp after including Rcpp.h #952

Closed
rstub opened this issue Mar 18, 2019 · 11 comments · Fixed by #959
Closed

Extending Rcpp after including Rcpp.h #952

rstub opened this issue Mar 18, 2019 · 11 comments · Fixed by #959

Comments

@rstub
Copy link
Contributor

rstub commented Mar 18, 2019

I learned something new on SO today: https://stackoverflow.com/a/55225006/8416610 Apparently it is possible to extend Rcpp with RCPP_EXPOSED_CLASS even after Rcpp.h has been included. Unsurprisingly @romainfrancois knew about that before: http://romainfrancois.blog.free.fr/index.php?post/2012/10/25/Rcpp-modules-more-flexible

Suggestion: Update the "Extending Rcpp" vignette to #include <Rcpp.h> instead of #include RcppCommon.h(sic) when discussing RCPP_EXPOSED_AS and RCPP_EXPOSED_WRAP. One might even draw attention to this quite useful feature.

@coatless
Copy link
Contributor

👍 Did you want to add the text @rstub ?

@eddelbuettel
Copy link
Member

It's more complicated than adding a line. It needs changes to two vignettes.

@rstub
Copy link
Contributor Author

rstub commented Mar 20, 2019

@eddelbuettel Which other vignette would have to be adjusted? The one on modules? Does this possibility to extend Rcpp require the usage of modules?

@eddelbuettel
Copy link
Member

I think so. The topic, as I understand your question, really combines 'extending' and 'modules'.

I am not so concerned about RcppCommon.h: we say in 'extending' why/where it is needed for forward declarations. Pretty much everywhere else we can use Rcpp.h. But I haven't had time to look.

Sketching out proposed changes here in a few bullet points may be a good start.

@rstub
Copy link
Contributor Author

rstub commented Mar 20, 2019

I initially thought that the minimal change would be to only change the code examples to

#include <Rcpp.h>
#include <foobar.h>

RCPP_EXPOSED_WRAP(Bar)

and

#include <Rcpp.h>
#include <foobar.h>

RCPP_EXPOSED_AS(Bar)

In addition, one could also add something like

Note that RCPP_EXPOSE_WRAP/AS/CLASS can be used after Rcpp.h haas been loaded.

to the discussion of these three macros.

However, right now I am unable to get a working example for RCPP_EXPOSED_CLASS without using modules. I guess I have to get some sleep first ...

@eddelbuettel
Copy link
Member

Yes, didn't mean to mislead. You need modules.

But as the topic is extending I realized we might as well put at least a forward reference in the other vignette.

@rstub
Copy link
Contributor Author

rstub commented Mar 21, 2019

You need modules.

Good, that makes it easier to tell a coherent story:

  • Remove the existing discussions on RCPP_EXPOSED_WRAP/AS/CLASS
  • Write a new section "Modules and Extension":
    • In some cases one needs as or wrap functionality for a class exposed via modules.
    • Instead of using the general methods for extension, one can use the RCPP_EXPOSED_WRAP/AS macros or alternatively the RCPP_EXPOSED_CLASS macro.
    • Do not use these macros together with the generic extension mechanisms.
    • These macros can be used after Rcpp.h has been loaded.
    • Some example, maybe similar to the SO question.
  • This section could go into the extension or the methods vignette. The other vignette would get a reference to it.

Comments? Preferences in which vignette the main text should go to?

@eddelbuettel
Copy link
Member

Why would we need to remove that section? RCPP_EXPOSED_* remains valid.

New section sounds good. along with example.

Placed in Modules vignette with reference from Extending vignette seems natural.

@rstub
Copy link
Contributor Author

rstub commented Mar 21, 2019

Problem with the current section(s) from my point of view is that they do not mention modules at all, which is why I tried to build something like

#include <RcppCommon.h>

class Foo {
public:
    Foo() = default;
};

RCPP_EXPOSED_CLASS(Foo)
#include <Rcpp.h>

// [[Rcpp::export]]
Foo getFoo() { Foo foo{}; return foo; }

// [[Rcpp::export]]
void handleFoo(Foo foo) { Rcpp::Rcout << "Got a Foo!" << std::endl; }

/***R
foo <- getFoo()
foo
handleFoo(foo)
*/

which does not work. So one would have to add something like "useful for classes exposed as Rcpp modules. See the Rcpp modules vignette for more details" into both sections.

@eddelbuettel
Copy link
Member

Can we make this more concrete by referencing which vignette and section you are talking about?

@rstub
Copy link
Contributor Author

rstub commented Mar 21, 2019

Extending Rcpp, ends of sections 2.2 and 3.2.

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

Successfully merging a pull request may close this issue.

3 participants