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

assign_in() NULL should assign rather than remove #636

Closed
lionel- opened this issue Feb 7, 2019 · 6 comments · Fixed by #919
Closed

assign_in() NULL should assign rather than remove #636

lionel- opened this issue Feb 7, 2019 · 6 comments · Fixed by #919

Comments

@lionel-
Copy link
Member

lionel- commented Feb 7, 2019

The modfied object returned by pluck<- should always have the same length structure.

@lionel-
Copy link
Member Author

lionel- commented Feb 8, 2019

Document difference with [[<-: no removal, no expansion.

@lionel-
Copy link
Member Author

lionel- commented Feb 27, 2019

After feedback from #634, maybe we should let puck create new elements — at least by name, it feels like assigning by index should be stricter (semantic difference between vector and associative containers).

This means we'd allow removing elements with zap(), but only by name?

What do you think @hadley?

@hadley
Copy link
Member

hadley commented Feb 27, 2019

I think it seems like a nice principle that pluck<-(x, i, value) have the same sizes and types as x. Assigning deeply into structures where bits might not exist seems risky to me and I think you'd be better off first normalising your data structure with some other tool.

@lionel-
Copy link
Member Author

lionel- commented Feb 27, 2019

In the shallow case it seems reasonable to do assign_in(x, "elt", value) and expect new elements to be added. In that case it's more of a $<- action than a [[<- one.

@hadley
Copy link
Member

hadley commented Feb 27, 2019

I think this needs more thought about what our desired $<- vs [[<- vs [<- semantics are

@hadley
Copy link
Member

hadley commented Aug 25, 2022

Reprex:

library(purrr)

x <- list(a = 1, b = list(c = 1))
pluck(x, "a") <- NULL
str(x)
#> List of 1
#>  $ b:List of 1
#>   ..$ c: num 1

pluck(x, "b", "c") <- NULL
str(x)
#> List of 1
#>  $ b: Named list()

Created on 2022-08-25 by the reprex package (v2.0.1)

Definitely feels like purrr is standardising on using zap() to delete.

@hadley hadley changed the title pluck-assign NULL should assign rather than remove assign_in() NULL should assign rather than remove Sep 6, 2022
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