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
with_seed() and with_preserve_seed() #45
Conversation
Codecov Report@@ Coverage Diff @@
## master #45 +/- ##
==========================================
+ Coverage 96.65% 96.87% +0.22%
==========================================
Files 13 14 +1
Lines 209 224 +15
==========================================
+ Hits 202 217 +15
Misses 7 7
Continue to review full report at Codecov.
|
R/seed.R
Outdated
#' the random seed at all (and also to avoid resetting it). | ||
#' @export | ||
with_seed <- function(seed, code) { | ||
if (is.null(seed)) { |
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.
I do not think we should have a NULL
case, if you don't want to do anything with the seed why would you call with_seed()
in the first place? (I realize this API simplifies the logic in the ggplot2 example)
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.
Agreed, will get rid of this special case. Do we need the NA case?
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.
I think not, maybe we could just include an example showing setting a random seed, e.g.
with_seed(sample.int(.Machine$integer.max, 1L), {
# ... some code
})
R/seed.R
Outdated
#' Pass `NA` to use the RNG to draw the random seed, pass `NULL` to not change | ||
#' the random seed at all (and also to avoid resetting it). | ||
#' @export | ||
with_seed <- function(seed, code) { |
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.
We should probably also expose the kind
and normal.kind
parameters to set.seed()
as well
Thanks. I've addressed your feedback. |
Thanks! |
Taken from tidyverse/ggplot2#1996
@hadley: PTAL.