fct_collapse produces wrong counts when group_other = TRUE

[hongcui]

Using the example given in the package.

Remove the grouping of Other Party, add group_other=TRUE, expect Other has a count of 'Other Party' 393, but result shows 3490.

library(tidyverse)
fct_count(gss_cat$partyid)
#> # A tibble: 10 x 2
#>    f                      n
#>    <fct>              <int>
#>  1 No answer            154
#>  2 Don't know             1
#>  3 Other party          393
#>  4 Strong republican   2314
#>  5 Not str republican  3032
#>  6 Ind,near rep        1791
#>  7 Independent         4119
#>  8 Ind,near dem        2499
#>  9 Not str democrat    3690
#> 10 Strong democrat     3490

partyid2 <- fct_collapse(gss_cat$partyid,
                         missing = c("No answer", "Don't know"),
                         rep = c("Strong republican", "Not str republican"),
                         ind = c("Ind,near rep", "Independent", "Ind,near dem"),
                         dem = c("Not str democrat", "Strong democrat"),
                         group_other = TRUE
)
fct_count(partyid2)
#> # A tibble: 5 x 2
#>   f           n
#>   <fct>   <int>
#> 1 missing   155
#> 2 rep      2707
#> 3 ind      8942
#> 4 dem      6189
#> 5 Other    3490

^{Created on 2019-08-08 by the reprex package (v0.3.0)}

[hadley]

Could you please rework your reproducible example to use the reprex package ? That makes it easier to see both the input and the output, formatted in such a way that I can easily re-run in a local session.

[hongcui]

Of course. Just did. Thank you!

[batpigandme]

OK, I'm digging into this one. Basically, we're corrently assigning the new names over the old factor levels as though they were in the order of the new levels (they're not — it's just that they happened to be in close enough order that some of the results were right, currently, it assumes that whatever the last item in your original list of factors is what you want to put into "Other").

Happens here:

forcats/R/collapse.R

Line 29 in fb11604

names(levs) <- names(new)[rep(seq_along(new), vapply(new, length, integer(1)))]

[batpigandme]

I'm closing this as a duplicate of #172, since both are fixed by #176.