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

Additional check functions #86

Merged
merged 11 commits into from
Jan 26, 2025
Merged

Additional check functions #86

merged 11 commits into from
Jan 26, 2025

Conversation

topepo
Copy link
Member

@topepo topepo commented Oct 20, 2024

The rlang type checkers lack a few functions. This PR has three: one for a single vector and two to check numeric vectors.

R/checks.R Outdated Show resolved Hide resolved
R/checks.R Show resolved Hide resolved
R/checks.R Show resolved Hide resolved
)
}

# check_logical <- function(x,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brulee has its own check_logical(). The is temporary to avoid bringing in a lot of snapshots into this PR.

expect_silent(brulee:::check_number_whole_vec(variable))
expect_silent(brulee:::check_number_whole_vec(variable[1]))

variable <- seq(0, 1, length.out = 3)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one demonstrates that you can get a whole number returned that is not an integer data type.

@topepo topepo marked this pull request as ready for review October 20, 2024 17:47
R/checks.R Outdated Show resolved Hide resolved
R/checks.R Outdated Show resolved Hide resolved
R/checks.R Show resolved Hide resolved
R/checks.R Show resolved Hide resolved
R/checks.R Outdated Show resolved Hide resolved
tests/testthat/test-checkers.R Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you rename either this file or the corresponding R file so that they have the same slug? I.e. either checks or checkers but not both.

R/checks.R Outdated Show resolved Hide resolved
tests/testthat/test-checkers.R Outdated Show resolved Hide resolved
tests/testthat/test-checkers.R Outdated Show resolved Hide resolved
tests/testthat/test-checkers.R Outdated Show resolved Hide resolved
R/checks.R Outdated Show resolved Hide resolved
R/checks.R Show resolved Hide resolved
R/checks.R Outdated Show resolved Hide resolved
@hfrick
Copy link
Member

hfrick commented Oct 21, 2024

Oh, we reviewed both! 👀

@hfrick
Copy link
Member

hfrick commented Oct 21, 2024

But at least we didn't contradict each other 😂

R/checks.R Outdated Show resolved Hide resolved
@EmilHvitfeldt
Copy link
Member

Worth noting, that a check_whole_numbers() function properly will get added to the standalone file eventually, once something implements it r-lib/rlang#1714 (comment)

@topepo topepo merged commit 577569a into main Jan 26, 2025
10 checks passed
@topepo topepo deleted the additional-check-functions branch January 26, 2025 19:22
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

Successfully merging this pull request may close these issues.

4 participants