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
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent stuff -- 🚢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second approval provided automatically after 24 hours. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
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. |
This handles #3486.
By putting the
mlpack
andcereal
headers intoinst/include/
, they will be installed as part of the R package installation process. I also had to updateMakevars
to correctly include the headers (in the same way that's done in theMakevars
ofRcppEnsmallen
).