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

Should pluck<- actually be chuck<-? #634

Closed
DavisVaughan opened this issue Feb 4, 2019 · 3 comments · Fixed by #919
Closed

Should pluck<- actually be chuck<-? #634

DavisVaughan opened this issue Feb 4, 2019 · 3 comments · Fixed by #919

Comments

@DavisVaughan
Copy link
Member

I am not sure that pluck() and pluck<-() are consistent. pluck() returns NULL if the element does not exist, but pluck<-() throws an error. In that way, pluck<-() feels more like the counterpart to chuck().

I am not sure that pluck<-() should even exist, since assign_in() is very strict and requires that the plucked-location must exist. I am just arguing that is should be named chuck<-() instead.

library(purrr)

pluck(mtcars, "y")
#> NULL

chuck(mtcars, "y")
#> Error: Can't find name `y` in vector

# should this be `chuck<-`()?
pluck(mtcars, "y") <- 1 
#> Error: Can't find name `y` in vector

# this should be `pluck<-`() I think
mtcars$y <- 1

# what I think should be `chuck<-`() uses chuck() already
`pluck<-`
#> function (.x, ..., value) 
#> {
#>     assign_in(.x, list2(...), value)
#> }
#> <bytecode: 0x7f9558ae34e8>
#> <environment: namespace:purrr>
purrr:::assign_in
#> function (x, where, value) 
#> {
#>     chuck(x, !!!where)
#>     call <- reduce_subset_call(quote(x), as.list(where))
#>     call <- call("<-", call, value)
#>     eval_bare(call)
#>     x
#> }
#> <bytecode: 0x7f9558ae2790>
#> <environment: namespace:purrr>

Created on 2019-02-04 by the reprex package (v0.2.1.9000)

@lionel-
Copy link
Member

lionel- commented Feb 4, 2019

We considered this and thought the interface would be simpler overall this way because casual users wouldn't need to learn the difference between the two and remember that they have to use chuck<- rather than pluck<-. Note that a lenient pluck<- isn't possible because there is no type information for non-existing locations.

@lionel- lionel- closed this as completed Feb 4, 2019
@lionel-
Copy link
Member

lionel- commented Feb 22, 2019

Just got some feedback from @jennybc about modify_in() and assign_in().

  • modify_in() might need an option to ignore non-existing locations instead of failing.
  • assign_in() might need an option to force assignment in non-existing locations (using lists for deep assignments where there is no type information? Or error in that case?).

The latter suggests we should have both pluck<- and chuck<-.

Also would be useful to have a can_pluck() predicate.

cc @hadley

@lionel- lionel- reopened this Feb 22, 2019
@lionel-
Copy link
Member

lionel- commented Feb 25, 2019

From @hadley:can_pluck() could be has_element()

hadley added a commit that referenced this issue Sep 7, 2022
* Can assign even when parent doesn't exist. Fixes #704.
* Can assign `NULL`. Fixes #636

Also fixes #634 since `pluck()` is now permissive.
@hadley hadley closed this as completed in c3ad48c Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants