-
Notifications
You must be signed in to change notification settings - Fork 6
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
Package fails to install under R 4.0 #456
Comments
Ohhh...goodspot @klmr !! @pharmaverse/admiral and @pharmaverse/admiral_comm ummm....how should we approach:
|
I'd vote for the second option - R 4.1 is quite old anyway. |
So should we handle this as an official bug fix with a re-release? |
I don't think so. The min R version is something that is not correct is many many many pkgs on CRAN. I would just fix in dev, and it'll be correct in the next release |
Could we add a CI/CD workflow which tries to install and test the package with the minimum versions stated in |
Perhaps we can just say somewhere on the website we support the last 2, 3, 4 (whatever we choose) versions of R, and supplement our workflows with the R CMD Check from Posit that makes it easy to check the last n versions of R. - {os: ubuntu-latest, r: 'devel', http-user-agent: 'release'}
- {os: ubuntu-latest, r: 'release'}
- {os: ubuntu-latest, r: 'oldrel-1'}
- {os: ubuntu-latest, r: 'oldrel-2'}
- {os: ubuntu-latest, r: 'oldrel-3'}
- {os: ubuntu-latest, r: 'oldrel-4'} |
We say we develop using the earliest of the last 3 versions of R, in documentation: |
We already have the Posit workflow file here, and we can just uncomment the lines.
|
ah i see - ummm... can we re-work the admiralci one since it is used in admiral as well as well as the other admiral packages. Just be nice to all pull from the same spot. I'm totally fine to simplify the one in admiralci! |
There it says
Is the part "and the closest snapshots of packages to those R versions" up to date? I remember vaguely that we changed our strategy to use the latest version of the packages for all R versions. |
I think testing more R versions doesn't solve the issue. If we had tested with 4.1, we still wouldn't have discovered the problem. If we can add a workflow which tests the package on the minimum R version and minimum package versions as specified in the Otherwise, I would rely on the developers and users that they report wrong minimum versions. |
My understanding is that we are saying in our current DESCRIPTION that admiral works with R version 4.0. However, we are using a pipe If we had been testing the package build on 4.0 I believe we would of found this issue out. Maybe we should just give a try to experiment. Anyways, I do think the minimum version we suggest in the DESCRIPTION should be tested no matter what! :) |
Yes this is possible. We have a dedicated GitHub Action that we use in NEST already. |
According to its
DESCRIPTION
, the package requires R 4.0. However, the following line uses the native pipe operator, which is only available from R 4.1 onwards:admiraldev/R/process_set_values_to.R
Line 96 in 86c007a
The text was updated successfully, but these errors were encountered: