-
Notifications
You must be signed in to change notification settings - Fork 113
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
Type checkers part 2 #1391
Type checkers part 2 #1391
Conversation
@@ -107,13 +107,7 @@ step_other <- | |||
objects = NULL, | |||
skip = FALSE, | |||
id = rand_id("other")) { | |||
if (!is_tune(threshold)) { |
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.
There's a lot of moving things here and in #1387. My rationale is that, if a the tuning process inserts a value, we would not check it. For example, finalize_recipe()
does use the constructor.
It does defer the checking until prep-time which is later than this but it seems like the only way to really be sure that we are getting corrrect inputs.
) | ||
expect_snapshot( | ||
rec %>% step_window(y1, size = pi) | ||
rec %>% step_window(y1, size = 3 + .Machine$double.eps) %>% prep() |
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.
So I don't know that we need both of the tests using .Machine$double.eps
but it did affirm my belief of what should happen given how all_equal()
works.
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.
amazing!
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex https://reprex.tidyverse.org) and link to this issue. |
Complement of #1387
This should do it for the new type checkers in this package.