CRAN packaging

[wolfv]

@DavisVaughan since you made a new package, this is basically a question for you!

We have a process in our CMake file that generates a CRAN package and we have uploaded versions of this package to CRAN. Inside the CRAN package we have copies of xtensor and xtl (we should add xsimd soon, as well).

Are we installing them in the wrong locations? I think it would be good if "our" CRAN package works out of the box – and stuff like Rcpp::sourceCpp(...) just works.

[DavisVaughan]

I think you've done most everything correctly.

• Using Imports and LinkingTo Rcpp in DESCRIPTION is good
• Putting the 3 header libraries in inst/include is correct.

A few points, and if @eddelbuettel has a few minutes it would be great to get his second opinion as he will be the expert.

1) ~I'm not sure that you need the inst/include/xtensor.h file? I think all of the headers are available just by including them in inst/include. Dirk's BH package is similar to this one in a lot of ways, and he doesn't have one of those and everything from inst/include/boost/ is available with just #include <boost/..submodule..>. https://github.com/eddelbuettel/bh~ I forgot that you have an example xtensor_r_example.cpp that generates RcppExports.cpp that does require that header file.

2) Regarding the reason I repackage things in xtensorrr, I use RStudio and can take full advantage of libclang's error messages if I have everything all together. Also, it's just easier for me to edit and work with personally when I don't have to use CMake to bundle it all together. I don't think many R users have much experience with that. But I totally get why you guys use it to easily keep all of the libraries in sync.

3) I can see that you've got # CXX_STD = CXX14 commented out in your Makevars. I'm not sure if it's "best practice" to only have PKG_CXXFLAGS = -I../inst/include --std=c++14 as a way of indicating that cpp14 is needed. The R Extensions manual suggests either CXX_STD or SystemRequirements: in the DESCRIPTION file. https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Using-C_002b_002b14-code (I have a feeling some of this is due to the hell that is caused by trying to use cpp14 with R on travis and their outdated g++. I too have had to deal with that crap and Dirk's post was helpful). It's clearly working as you got on CRAN without it, but I thought you might need one or the other.

4) Regarding Rcpp::sourceCpp() and the close companion Rcpp::cppFunction(), I think you are close, but it doesn't quite work out of the box yet because of an issue with a conflict when including Rcpp and rarray.hpp in the "wrong" order.

// [[Rcpp::depends(xtensor)]]
// [[Rcpp::plugins(cpp14)]]

#include <xtensor-r/rarray.hpp>
#include <Rcpp.h>

// [[Rcpp::export]]
SEXP rray_add_cpp(const xt::rarray<double>& x, const xt::rarray<double>& y) {
  const xt::rarray<double>& res = x + y;
  return res;
}

all good, no issues.

// [[Rcpp::depends(xtensor)]]
// [[Rcpp::plugins(cpp14)]]

#include <Rcpp.h>
#include <xtensor-r/rarray.hpp>

// [[Rcpp::export]]
SEXP rray_add_cpp(const xt::rarray<double>& x, const xt::rarray<double>& y) {
  const xt::rarray<double>& res = x + y;
  return res;
}

In file included from testrcpp.cpp:5:
In file included from /Library/Frameworks/R.framework/Versions/3.5/Resources/library/xtensor/include/xtensor-r/rarray.hpp:18:
In file included from /Library/Frameworks/R.framework/Versions/3.5/Resources/library/xtensor/include/xtensor/xcontainer.hpp:22:
/Library/Frameworks/R.framework/Versions/3.5/Resources/library/xtensor/include/xtensor/xmath.hpp:36:28: error: expected member name or ';' after declaration specifiers
        static constexpr T PI = 3.141592653589793238463;
        ~~~~~~~~~~~~~~~~~~ ^
/Library/Frameworks/R.framework/Resources/include/R_ext/Constants.h:36:24: note: expanded from macro 'PI'
#define PI             M_PI
                       ^
/usr/include/math.h:692:21: note: expanded from macro 'M_PI'
#define M_PI        3.14159265358979323846264338327950288   /* pi             */
                    ^
1 error generated.
make: *** [testrcpp.o] Error 1
Error in Rcpp::sourceCpp("Desktop/testrcpp.cpp") : 
  Error 1 occurred building shared library.

There is some interesting interaction of the PI constant between xtensor and core R. Rcpp::cppFunction() will force the inclusion of Rcpp.h first before anything else is included so the same error shows up there. I don't know what is happening.

[eddelbuettel]

@wolfv: You may recall that I tried to help you good folks a while back. In essence there are two major usage modes you are conflating here: ad-hoc (cppFunction(), sourceCpp()) and packages. The latter do not use the former.

[wolfv]

@eddelbuettel My goal is that R-users can just do packages.install("xtensor") and get everything to work, from small snippets imported with sourceCpp to building other packages on top of xtensor. Does that sound doable, or not?

We will (forever) have a problem with installing with devtools from github since we're not going to add the xtensor headers to this repository.

I'll try to iron that out in a container since I actually do have xtensor headers installed in my system so there are interactions.

[SylvainCorlay]

We will (forever) have a problem with installing with devtools from github since we're not going to add the xtensor headers to this repository.

Yeah, we build the CRAN tarball as a cmake target, and no generated file is versioned in the repository.

[eddelbuettel]

@wolfv, @SylvainCorlay : you are mixing two conversations. It;s better to separate them. Wolf describe a usage outcome. We can talk about that. Sylvain (needlessly, if I may add) reminds everyone that he likes CMake. Which is cool and nobody minds that. But nothing in the R stack uses it (by default) so this is orthogonal. You can organize your source repo and process any way you like. But R CMD INSTALL .... for some third party package can in general NOT assume CMake to be present. It is simply not part of the default R stack. You can of course add more requirements...

@wolfv: "Does that sound doable, or not?" It does to me, and the devil is in the detail. But Rcpp facilitates dozens of packages doing that -- providing their headers for other packages, and for use by sourceCpp() and cppFunction(). Happy to help over on rcpp-devel or other places if you have questions. There should be dozens to examples to crib from.

[DavisVaughan]

This works "out of the box" so I'm fairly certain the only issue is with the PI constant problem that arises when you switch the order of the includes. Then cppFunction() should also work. Everything is fine in packages because I can control the include order.

install.packages("xtensor")
#> 
#> The downloaded binary packages are in
#>  /var/folders/41/qx_9ygp112nfysdfgxcssgwc0000gn/T//RtmpCPDFdY/downloaded_packages

Rcpp::sourceCpp(
  code = "
  // [[Rcpp::depends(xtensor)]]
  // [[Rcpp::plugins(cpp14)]]

  #include <xtensor-r/rarray.hpp>
  #include <Rcpp.h>

  // [[Rcpp::export]]
  SEXP rray_add_cpp(const xt::rarray<double>& x, const xt::rarray<double>& y) {
    const xt::rarray<double>& res = x + y;
    return res;
  }
  ")

x <- matrix(as.double(1:10), ncol = 2)
y <- matrix(as.double(1:5))
rray_add_cpp(x, y)
#>      [,1] [,2]
#> [1,]    2    7
#> [2,]    4    9
#> [3,]    6   11
#> [4,]    8   13
#> [5,]   10   15

[SylvainCorlay]

Regarding Rcpp::sourceCpp() and the close companion Rcpp::cppFunction(), I think you are close, but it doesn't quite work out of the box yet because of an issue with a conflict when including Rcpp and rarray.hpp in the "wrong" order.

@DavisVaughan could you try applying the following patch locally and tell me if this fixes the issue?

https://github.com/QuantStack/xtensor/pull/1264

The R headers are defining a PI macro and we are trying to prevent the macro expansion.

[DavisVaughan]

It doesn't seem to work (xtensorrr is using the xtensor PR):

// [[Rcpp::depends(xtensorrr)]]
// [[Rcpp::plugins(cpp14)]]

#include <Rcpp.h>
#include <xtensor-r/rarray.hpp>

// [[Rcpp::export]]
SEXP rray_add_cpp(const xt::rarray<double>& x, const xt::rarray<double>& y) {
  const xt::rarray<double>& res = x + y;
  return res;
}

In file included from testrcpp.cpp:5:
In file included from /Library/Frameworks/R.framework/Versions/3.5/Resources/library/xtensorrr/include/xtensor-r/rarray.hpp:18:
In file included from /Library/Frameworks/R.framework/Versions/3.5/Resources/library/xtensorrr/include/xtensor/xcontainer.hpp:22:
/Library/Frameworks/R.framework/Versions/3.5/Resources/library/xtensorrr/include/xtensor/xmath.hpp:37:29: error: expected member name or ';' after declaration specifiers
        static constexpr T (PI) = 3.141592653589793238463;
        ~~~~~~~~~~~~~~~~~~  ^
/Library/Frameworks/R.framework/Resources/include/R_ext/Constants.h:36:24: note: expanded from macro 'PI'
#define PI             M_PI
                       ^
/usr/include/math.h:692:21: note: expanded from macro 'M_PI'
#define M_PI        3.14159265358979323846264338327950288   /* pi             */
                    ^
In file included from testrcpp.cpp:5:
In file included from /Library/Frameworks/R.framework/Versions/3.5/Resources/library/xtensorrr/include/xtensor-r/rarray.hpp:18:
In file included from /Library/Frameworks/R.framework/Versions/3.5/Resources/library/xtensorrr/include/xtensor/xcontainer.hpp:22:
/Library/Frameworks/R.framework/Versions/3.5/Resources/library/xtensorrr/include/xtensor/xmath.hpp:37:29: error: expected ')'
/Library/Frameworks/R.framework/Resources/include/R_ext/Constants.h:36:24: note: expanded from macro 'PI'
#define PI             M_PI
                       ^
/usr/include/math.h:692:21: note: expanded from macro 'M_PI'
#define M_PI        3.14159265358979323846264338327950288   /* pi             */
                    ^
/Library/Frameworks/R.framework/Versions/3.5/Resources/library/xtensorrr/include/xtensor/xmath.hpp:37:28: note: to match this '('
        static constexpr T (PI) = 3.141592653589793238463;
                           ^
2 errors generated.
make: *** [testrcpp.o] Error 1

switching the order of the includes again works.

[wolfv]

Ok, too bad .. the only other option is to store the macro in a temp macro, use #undef PI and then re-define it from the temporary macro below :/

[DavisVaughan]

Would defining STRICT_R_HEADERS have fixed the issue?

https://github.com/wch/r-source/blob/5a156a0865362bb8381dcd69ac335f5174a4f60c/src/include/R_ext/Constants.h#L35

There's an issue for this over in Rcpp https://github.com/RcppCore/Rcpp/issues/898

[wolfv]

Sure, that would definitely help. I have this code right now:

    // Unfortunately R defines a PI macro without any prefix.
    // So we undef it here, store it in a temporary, and redefine it after
    // this block.
    #ifdef PI
    #define XTEMP_PI PI
    #undef PI
    #endif

    template <class T = double>
    struct numeric_constants
    {
        static constexpr T PI = 3.141592653589793238463;
        ...
    };

    #ifdef TEMPPI
    #define PI TEMPPI
    #undef XTEMP_PI
    #endif

[DavisVaughan]

I think Rcpp would have to use STRICT_R_HEADERS and it wouldn't be enough for xtensor to do it. But I guess that works too.

[eddelbuettel]

You can do

#define STRICT_R_HEADERS
#include <Rcpp.h>

and/or define/undefine as needed. It happens with other codebases at times.

[SylvainCorlay]

We will need to protect against cases where R headers or Rcpp are included without STRICT_R_HEADERS so we will probably end up locally undefining the poluting macros in xtensor.

[DavisVaughan]

And you still can't use Rcpp::cppFunction() since using namespace Rcpp; is one of the first things that get's spliced in. Maybe strict_headers = FALSE would be a nice argument to that, but that's just me thinking out loud.

[eddelbuettel]

Easy. Just #define STRICT_R_HEADERS at your level.

It's an R thing we too try to protect against as well but when we tried to turn it on too many things fell over so we are trying to more gradually bring it in.

[SylvainCorlay]

Even if we define that macro, client code may have included RCPP (or the R headers themselves) unfortunately.

[eddelbuettel]

Well if you define it there won't be spillage when folks include R headers after your.

And for cases where people include R headers before your (maybe via Rcpp [btw: we spell it Capital-R lowercase cpp ]) you have the option to test for their headers.

[SylvainCorlay]

Unfortunately, the undefining / redefining of macros does not really allow redefining

[SylvainCorlay]

We will (forever) have a problem with installing with devtools from github since we're not going to add the xtensor headers to this repository.

Yeah, we build the CRAN tarball as a cmake target, and no generated file is versioned in the repository.

[...] We can talk about that. Sylvain (needlessly, if I may add) reminds everyone that he likes CMake. Which is cool and nobody minds that. But nothing in the R stack uses it (by default) so this is orthogonal. You can organize your source repo and process any way you like. But R CMD INSTALL .... for some third party package can in general NOT assume CMake to be present. It is simply not part of the default R stack. You can of course add more requirements...

Might point is not about cmake, that is applicable regardless of the build system: We don't vendor dependencies in git repositories, and try to avoid checking in generated content.

The tarball generated by the cran cmake target (which is uploaded to cran) can be installed with R CMD INSTALL ... and does not require cmake.

[eddelbuettel]

@Sylvain Yes, I got that lecture from you a few times, whether I asked for it or not :) I didn't say anything about repo or modules or whatever. Do what you want or need to do. Your repo.

Now, I look after Rcpp. I check against all (by now 1500+) revdeps prior to releases. I do not think we have a single one biting with PI because if it were CRAN would not let us on. Myself, I think I switched to only ever using M_PI many many years ago so I am sure things can be worked out here too. I'll unsubscribe from the thread as there does not seem to be a need for any particular Rcpp help here. Looking forward to seeing from xtensor and xtensor R uses. Cheers.

[wolfv]

Dirk, no worries, I think we went a bit on a tangent here. The CMake stuff is just an implementation detail for us to make our package look like a R package before uploading. It could be a Python or R script as well. Totally understood that CMake is not the tool of choice for the R community. If we had something better that was as widely adopted in the C++ community we would have switched yesterday, it's that bad.

Of course the PI thing will not resolve by tomorrow in Rcpp, and you've already stated that you want to enable the strict headers in ~a year, so that's great for us.

In the meantime we need to decide wether we want to workaround this by putting some ifdef/undef guards in xtensor (the above example from @SylvainCorlay contains a typo, that's why it doesn't work) or wether we just want to clearly document the behavior and tell everyone to either switch the order of includes or use the strict header macro.

Yep! Looking forward to xtensor + Rcpp uses as well! If there is something I am not getting from the R world I'll head over to the Rcpp channel. Thanks for your time!

[DavisVaughan]

@wolfv I think that even with the fixed typo it doesn't work. I tried but get different errors. Maybe I have a typo too.

---

Two things I was able to get working are:

---

and

But they are rather ugly...

[wolfv]

I've created a branch in my fork called package that contains the complete, unpacked R package that we would normally upload to CRAN.

That makes it possible to get started quickly with devtools:

devtools::install_github("wolfv/xtensor-r", ref="package")

https://github.com/wolfv/xtensor-r/tree/package

@SylvainCorlay @DavisVaughan thoughts?

[DavisVaughan]

This is essentially the idea behind why I have xtensorrr. I think this would be useful. Even more useful would probably be the ability to install the development version like this at any time. If Travis could build the unpacked package and commit it to a "development" branch in xtensor-r, that would probably work. We do a similar idea with automatically committing R package documentation updates straight to gh-pages every time there is a passing commit to master, which gets rendered as a website.

[SylvainCorlay]

@DavisVaughan we have made some progress on this.

You can check out the https://github.com/QuantStack/Xtensor.R repository.

• It is installable from GitHub with devtools' install_github. The resulting package will xtl, xtensor, xtensor-r by default (which are cloned upon installation).
• Installing with --configure-args='--novendor' will prevent the vendoring these dependencies at the installation stage.
• We can still use it to generate a vendoring tarball for cran. (run ./vendor then R CMD build .) so that the CRAN package can be installed without requiring git,
• The conda-forge package will depend on the conda packages for xtensor, xtensor-r, xtl etc. The conda recipe will do R CMD INSTALL --configure-args='--novendor'.

Let me know what you think!

[SylvainCorlay]

Now the Xtensor.R repository is

• installable from GitHub
• supports vendoring and non-vendoring cases
• will be the primary source for the R package and the CRAN tarball,

while the xtensor-r only holds the pure C++ header-only library.

Besides, the R package for xtensor has been packaged for conda-forge, and properly depends on xtensor, xtensor-r, xtl instead of vendoring them. Closing the issue!

[DavisVaughan]

This is great progress. Its on my calendar to give it a try tomorrow. I think this is a really important step. Thanks for the hard work here!