Add by to chop()

[hrryt]

Fixes #1490
Fixes #1506

Snags:

1. In the absence of both cols and by, should we 'chop everything' as nest() does? I have thrown an error instead.
2. Should chop() use group vars if data is grouped, as nest() does? Would this require a generic?
3. What should happen if a column is selected by both cols and by? For now, the name is duplicated and repaired.

An example for the latter point:

df <- tibble(x = c(1, 1, 1, 2, 2), y = c(2, 1, 2, 3, 4), z = 1:5)
nest(df, data = everything(), .by = y)
#> # A tibble: 4 × 2
#>       y data            
#>   <dbl> <list>          
#> 1     2 <tibble [2 × 3]>
#> 2     1 <tibble [1 × 3]>
#> 3     3 <tibble [1 × 3]>
#> 4     4 <tibble [1 × 3]>
chop(df, everything(), by = y)
#> New names:
#> • `y` -> `y...1`
#> • `y` -> `y...3`
#> # A tibble: 4 × 4
#>   y...1           x       y...3           z
#>   <dbl> <list<dbl>> <list<dbl>> <list<int>>
#> 1     2         [2]         [2]         [2]
#> 2     1         [1]         [1]         [1]
#> 3     3         [1]         [1]         [1]
#> 4     4         [1]         [1]         [1]

[DavisVaughan]

Regarding the above snags:

In the absence of both cols and by, should we 'chop everything' as nest() does? I have thrown an error instead.

I believe that is correct. I have kept this behavior.

Should chop() use group vars if data is grouped, as nest() does? Would this require a generic?

I believe changing this behavior of chop() is out of scope for this PR. chop() currently does not try and respect existing grouping columns in any way (like, in theory it could chop by the grouping columns if no inputs are provided, but I don't think that feels that useful, I'd rather people be explicit).

What should happen if a column is selected by both cols and by? For now, the name is duplicated and repaired.

This is now an informative error. It now operates the same way as pivot_wider() with id_cols, where the id_cols tidyselection is computed after removing names_from and values_from columns from the data. Similarly, cols is computed after removing any by cols from the data. This:

• Disallows df |> chop(x, by = x), forcing an informative error
• Enables df |> chop(everything(), by = x) to be meaningful

I'd argue this aligns with pivot_wider() and with how across() works

[DavisVaughan]

Perhaps in a perfect world I'd move cols after the empty ... now that it default to NULL, but that would probably not be worth the effort

Let me know if this feels too awkward as is to you

[hadley]

I think it's worth considering? It's not too much effort to provide a deprecation message when !missing(...1)?

[hadley]

@DavisVaughan anything in particular that you're looking for from my review?

[DavisVaughan]

@hadley I have left 3 comment blocks open above. Just take a look at those, I'm confident in the implementation

[hadley]

I really like filling in this little gap in the tidyverse ecosystem!

[hadley]

I think it's worth considering? It's not too much effort to provide a deprecation message when !missing(...1)?

[DavisVaughan]

Thanks for getting this started @hrryt!