Changes list_modify to be able to remove key-value pairs whose value is a list

[adamroyjones]

The documentation to list_modify indicates that one should "[u]se zap() to remove values" from a list. This does not work in the expected way when attempting to remove a key-value pair whose value is a list:

library(purrr)

list(foo = 1, bar = 2) %>% list_modify(foo = zap())

list(foo = c(1, 2), bar = 3) %>% list_modify(foo = zap())

list(foo = list(fst = 1, snd = 2), bar = 3) %>% list_modify(foo = zap())

The first two examples above return an object identical to list(bar = 3); the third is such that the call to list_modify is a no-op. This pull request fixes this behaviour. It also adds a unit test to cover this case.

Issues: #776

[lionel-]

Looks good, can you just remove the extra whitespace changes please?

[adamroyjones]

I've removed the whitespace changes (but it's a little unclear whether I need to re-raise the request; this is the first time I've opened a (real) PR in GitHub, so please let me know if I've done something wrong).

I'm quite happy to contribute more of my time to purrr (and the tidyverse more broadly)—is there a register of issues, or something similar?

[lionel-]

No need to reopen the pull request as this would just add noise. Pushing the changes directly in the pull request is the way to go.

You also need to add a NEWS bullet explaining your change and linking to your github handle and the relevant github issue. I've done that now.

is there a register of issues, or something similar?

We sometimes have "help wanted" or "good first contribution" labels on github issues.

Thanks for the fix!

[BrianDiggs]

I would like to recommend an additional test that fails on the current release but should be fixed by this patch:

expect_equal(list_modify(list(list(1,2), 2, 3), zap(), zap()), list(3))

This test exercises the branch of code which is used for replacing a list when the arguments are not named. It parallels the added test in this pull request for named arguments.