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

Implemented accumulate for recursive folding #145

Merged
merged 5 commits into from Dec 7, 2015

Conversation

@michaelquinn32
Copy link
Contributor

@michaelquinn32 michaelquinn32 commented Dec 5, 2015

Following up on #46 and #11, I implemented a version of accumulate, which adds recursive folding to the package.

The differences between accumulate and reduce are minor. I simply added another argument (accumulate = TRUE) to the call of Reduce. Nonetheless, this change provides a lot of utility.

I have two examples in the documentation. The first shows flooring and capping a dataset, which is an extension of an example in Haskell's scanl documentation. The second example shows a simple simulation of a random walk with drift.

#'
#' rerun(5, rnorm(100)) %>% setNames(paste0("sim", 1:5)) %>%
#' map(~ accumulate(., function(.x, .y) .05 + .x + .y)) %>%
#' map(~ data.frame(value = .x, step = 1:100)) %>%

This comment has been minimized.

@hadley

hadley Dec 6, 2015
Member

This could now be map_df(~ data_frame(value = .x, step = 1:100), .id = "simulation")

#' map(~ accumulate(., function(.x, .y) .05 + .x + .y)) %>%
#' map(~ data.frame(value = .x, step = 1:100)) %>%
#' bind_rows(. , .id = "simulation") %>%
#' ggplot(., aes(x = step, y = value)) + geom_line(aes(color = simulation))

This comment has been minimized.

@hadley

hadley Dec 6, 2015
Member

You don't need the explicit . here

#' library(dplyr)
#' library(ggplot2)
#'
#' rerun(5, rnorm(100)) %>% setNames(paste0("sim", 1:5)) %>%

This comment has been minimized.

@hadley

hadley Dec 6, 2015
Member

Can you put this on a new line and use set_names()?

#' ggplot(., aes(x = step, y = value)) + geom_line(aes(color = simulation))
#' }
accumulate <- function(.x, .f, ..., .init) {
force(.f)

This comment has been minimized.

@hadley

hadley Dec 6, 2015
Member

I think you should use as_function() here to be consistent with the rest of purrr.

@hadley
Copy link
Member

@hadley hadley commented Dec 6, 2015

Looks good - thanks! Can you please respond to my super nitpicky comments and add a bullet to NEWS?

@michaelquinn32
Copy link
Contributor Author

@michaelquinn32 michaelquinn32 commented Dec 6, 2015

Super nitpicky is super helpful! I didn't notice the addition of map_df. That's a great little tool.

I also did a little additional formatting to the examples. Hopefully they'll be a little easier to read. In retrospect, one of my examples didn't illustrate what I expected (the flooring and capping). It's dropped.

I also went ahead and added as_function() for all four. Please let me know if I missed anything.

hadley added a commit that referenced this pull request Dec 7, 2015
Implemented accumulate for recursive folding
@hadley hadley merged commit 6d4efbe into tidyverse:master Dec 7, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hadley
Copy link
Member

@hadley hadley commented Dec 7, 2015

Looks great - thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants