-
Notifications
You must be signed in to change notification settings - Fork 21
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
Issue #809 - Replace is.logical by isTRUE #815
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #815 +/- ##
=======================================
Coverage 95.54% 95.54%
=======================================
Files 21 21
Lines 1570 1573 +3
=======================================
+ Hits 1500 1503 +3
Misses 70 70 ☔ View full report in Codecov by Sentry. |
I think historically we didn't enforce r 3.5 or something like that. Or I
was stupid:D
…On Sun, 19 May 2024, 15:04 James Azam, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In R/check-input-helpers.R
<#815 (comment)>
:
> - return(is.logical(check))
+ return(isTRUE(check))
Mmhhh interesting! Why was is.logical() use here? it returns TRUE for
both TRUE and FALSE whereas isTRUE will only returns TRUE for a value of
TRUE.
—
Reply to this email directly, view it on GitHub
<#815 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJBYFLLMQFPSQP4I4AWLSQTZDCPOLAVCNFSM6AAAAABH6EUGM6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANRVGA3TKMRTGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
I think it is unlikely to have caused any bugs unless in edge cases that I can't imagine atm, since "check" is either a string or |
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.
Jumping here to review this.
LGTM.
Lovely, thank you! |
Description
This PR closes #809.
Doe what it says on the tin. Purpose is improving robustness and code clarity.
Checklist
lintr::lint_package()
to check for style issues introduced by my changes.I have added a news item linked to this PR.