Closed andrjohns closed 1 year ago
There are a lot of whitespace changes due to my IDE stripping trailing whitespace, so it might be easier to review with the whitespace changes filtered out: https://github.com/stan-dev/rstanarm/pull/574/files?diff=split&w=1
Once this merged into the master branch, I'll work with @sambrilleman on integrating with the survival
branch so that we can get that ready for release as well
Thanks @andrjohns! We've been wanting to move to using rstantools for this for a while! I don't see anything that immediately jumps out to me as problematic, but this is more in @bgoodri's wheelhouse than mine, so we should have him review this too. @bgoodri Can you take a look at this when you have a chance? It would be great to merge this soon so that so that @andrjohns can work with @sambrilleman on getting the survival branch ready.
Also @andrjohns I just added you to the r-packages team in the stan-dev organization so you can more easily contribute to whichever R package you want (thanks for all the help with the R packages!). You should be able to make branches and do maintenance stuff, etc. (not that you have to!).
I'll look at it. The rstanarm package is hard because it has to be compiled with LTO on Windows.
@bgoodri any chance you've had time to have a look at this? It would be great to get 2.26+ compatibility in-place for rstanarm
I haven't yet, but I will after I deal with some final exam stuff
On Mon, Dec 12, 2022 at 11:29 AM Andrew Johnson @.***> wrote:
@bgoodri https://github.com/bgoodri any chance you've had time to have a look at this? It would be great to get 2.26+ compatibility in-place for rstanarm
— Reply to this email directly, view it on GitHub https://github.com/stan-dev/rstanarm/pull/574#issuecomment-1346834875, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ2XKTAB56AMOZKARIYFVLWM5HGJANCNFSM6AAAAAAQMHYL64 . You are receiving this because you were mentioned.Message ID: @.***>
@bgoodri Would you still have time to look at this? We'll need it for 2.26+
I have been looking at the ones for other packages, and since this one is similar, I am sure it will be fine. I am a little bit concerned about pending pull requests that branched off rstanarm a long time ago. I guess it could be managed but when those get merged, they might put stuff in the old locations or restore deleted files.
I have been looking at the ones for other packages, and since this one is similar, I am sure it will be fine. I am a little bit concerned about pending pull requests that branched off rstanarm a long time ago. I guess it could be managed but when those get merged, they might put stuff in the old locations or restore deleted files.
@bgoodri if you're happy with this PR would you mind merging and putting together a new CRAN release? I'm happy to update/check the current pending PR's for any conflicts
Closing in favour of #587, will reopen once that PR is merged and I've resolved the merge conflicts with this one
This PR updates the
rstanarm
installation to fully delegate torstantools
. This will help ensure ongoing compatibility withrstan
and its updates, and simplify some of the maintenance involved with the package.The PR also updates the Github actions workflow to test against more combinations of R and OS versions