Install mlpack and cereal headers in R package #3488
Conversation
|
Just got in from 25 miles on the bicycle where I realized ... we are doing this likely wrong :) When you install the headers into |
eddelbuettel
left a comment
eddelbuettel
left a comment
There was a problem hiding this comment.
Excellent stuff -- 🚢
![mlpack-bot[bot]](https://avatars.githubusercontent.com/in/23934?s=60&v=4){kind=small}
|
Yep, it took me a minute to debug the failing test (unrelated to this PR), but now that it's all green I'll go ahead and merge. 👍 |
|
Thanks so much for the persistence! Should we make a 4.1.1 release for CRAN? |
|
I was thinking of waiting on #3487 and other nearly-ready-to-go PRs to be merged, and then releasing the next version of mlpack (thus updating CRAN too). So, within the next week or so? |
|
That works too and is a wee bit cleaner :) |
@rcurtin Suggest to label the new release as mlpack 4.2, instead of 4.1.1. The changes are more than just bug fixes -- there are new features. On top of that, people like new shiny things with bigger numbers. |
|
Agreed, 4.2.0 is just fine given the new features. |
4 participants
This handles #3486.
By putting the
mlpackandcerealheaders intoinst/include/, they will be installed as part of the R package installation process. I also had to updateMakevarsto correctly include the headers (in the same way that's done in theMakevarsofRcppEnsmallen).