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

prepend: fails for empty list #637

Closed
czeildi opened this issue Feb 10, 2019 · 1 comment · Fixed by #687
Closed

prepend: fails for empty list #637

czeildi opened this issue Feb 10, 2019 · 1 comment · Fixed by #687

Comments

@czeildi
Copy link
Contributor

@czeildi czeildi commented Feb 10, 2019

purrr::prepend fails for empty list due to the checking of the before param though (partly based on the documentation) I would expect it to work as base::append by default. Is there a reason for this?

If you agree that it should work for empty lists as well I am happy to submit a PR.

purrr::prepend(list(), "a")
#> Error in purrr::prepend(list(), "a"): before > 0 && before <= n is not TRUE

append("a", list())
#> [[1]]
#> [1] "a"

purrr::prepend(list("b"), "a")
#> [[1]]
#> [1] "a"
#> 
#> [[2]]
#> [1] "b"
append("a", list("b"))
#> [[1]]
#> [1] "a"
#> 
#> [[2]]
#> [1] "b"

Created on 2019-02-10 by the reprex package (v0.2.0).

My first idea for implementation:

function (x, values, before = 1) 
{
    n <- length(x)
    stopifnot(n == 0 || (before > 0 && before <= n))
    if (before == 1) {
        c(values, x)
    }
    else {
        c(x[1:(before - 1)], values, x[before:n])
    }
}
@lionel- lionel- added the bug label Feb 27, 2019
@lionel-
Copy link
Member

@lionel- lionel- commented Feb 27, 2019

Maybe before should have NULL as default argument? Then it'd mean to prepend at the beginning, even in the 0 case? If 1 is passed instead, we'd still get an error as currently happens because this means the user assumptions regarding the input vector are not fulfilled.

A PR would be great!

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

Successfully merging a pull request may close this issue.

2 participants