Skip to content
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

_R_CHECK_LENGTH_1_CONDITION_ not needed after R 4.2.0 #3177

Open
infotroph opened this issue Jun 3, 2023 · 6 comments
Open

_R_CHECK_LENGTH_1_CONDITION_ not needed after R 4.2.0 #3177

infotroph opened this issue Jun 3, 2023 · 6 comments

Comments

@infotroph
Copy link
Member

infotroph commented Jun 3, 2023

User-visible changes in R 4.2 included:

Calling if() or while() with a condition of length greater than one gives an error rather than a warning. Consequently, environment variable _R_CHECK_LENGTH_1_CONDITION_ no longer has any effect.

In a future version bump when we stop testing R < 4.2.0, we can stop setting _R_CHECK_LENGTH_1_CONDITION_ in ci.yml and elsewhere.

@Sweetdevil144
Copy link
Contributor

Sweetdevil144 commented Jan 28, 2024

I guess the following checks are present only in .github/workflows/ci-weekly.yml and .github/workflows/ci.yml.
Also, I guess the behavior controlled by R_CHECK_LENGTH_1_CONDITION is now the default behavior, and thus the environment variable no longer has any effect.
What are your views regarding this @infotroph ?

@infotroph
Copy link
Member Author

It's the default from R 4.2.0 onward, but since we still support R 4.1 (and in fact that's the main version we test on!), I don't think we're ready to remove these yet.

@harshagr70
Copy link
Contributor

Hi @infotroph
I wanted to follow up on this issue . Given that R 4.2+ is now widely used, is the project still actively supporting R 4.1?
If not, I’d be happy to help remove "R_CHECK_LENGTH_1_CONDITION " from the CI configurations. Let me know your thoughts!

Thanks!

@infotroph
Copy link
Member Author

Thanks for checking, @harshagr70. We're still using R 4.1 as the default for our Docker base image, so we should change that before removing this.

Bumping the supported R version forward has been on my list for a while, but I've been waiting to do it until we get some lingering CI issues ironed out -- the issues aren't version-related, but they mean we've been manually approving all merges. A version bump is the kind of thing I'd feel a lot better doing when I can see the CI status staying green through the whole process.

@harshagr70
Copy link
Contributor

Thanks for the update, @infotroph! That makes sense. If there’s anything I can assist with—whether it’s related to the CI issues or the version bump—please let me know. I’d be happy to help!

@Sweetdevil144
Copy link
Contributor

@infotroph bumping from R4.1 to R4.2 would be a whole different project in itself. Although I think it may be really necessary because we too are getting outdated. I support this idea on a larger timeframe only.

Also, somethings needs to be done for the manual approval part :) , Although CI status are still not clearly green all the way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants