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

fct_collapse() should allow other_level = NA #291

Closed
1pakch opened this issue Dec 10, 2021 · 4 comments · Fixed by #337
Closed

fct_collapse() should allow other_level = NA #291

1pakch opened this issue Dec 10, 2021 · 4 comments · Fixed by #337
Labels
feature a feature request or enhancement help wanted ❤️ we'd love your help!
Milestone

Comments

@1pakch
Copy link

1pakch commented Dec 10, 2021

IMO the fct_collapse() function should support using NA as the other_level argument.

As of 0.5.0 one cannot use neither NA to NA_character_

library(purrr)
fac <- c(1, 2, 3, 4) %>% factor()
forcats::fct_collapse(fac, A = c('1', '2'), other_level = NA_character_)
forcats::fct_collapse(fac, A = c('1', '2'), other_level = NA)

and the workaround is quite awkward and too long

(
    fac
    %>% forcats::fct_collapse(A = c('1', '2'), other_level = 'NA')
    %>% na_if('NA')
    %>% fct_drop()
)

I'd be happy to look into it and make a PR.
This discussion might be relevant #101

@xxspurs

This comment was marked as off-topic.

@tidyverse tidyverse temporarily blocked xxspurs Mar 2, 2022
@hadley hadley added the feature a feature request or enhancement label Mar 2, 2022
@hadley
Copy link
Member

hadley commented Mar 2, 2022

Reprex:

library(forcats)
x <- factor(1:4)
fct_collapse(x, A = c("1", "2"), other_level = "other")
#> [1] A     A     other other
#> Levels: A other
fct_collapse(x, A = c("1", "2"), other_level = NA)
#> Error in new[[other_level]] <- levels[!levels %in% levs]: attempt to select less than one element in integerOneIndex

Created on 2022-03-02 by the reprex package (v2.0.1)

@hadley hadley added the help wanted ❤️ we'd love your help! label Mar 2, 2022
@hadley hadley added this to the v1.0.0 milestone Jan 3, 2023
@hadley hadley changed the title Supporting NA for other_level in fct_collapse() fct_collapse() should allow other_level = NA Jan 3, 2023
@hadley
Copy link
Member

hadley commented Jan 3, 2023

If we support this, also need to make sure it works in fct_lump_*() and fct_other().

fct_collapse() will need a decent amount of work to achieve this since it currently relies on fct_recode(). So maybe it make more sense to add a new fct_implicit_na() specifically for this purpose?

@hadley
Copy link
Member

hadley commented Jan 3, 2023

Hmmm, rather surprisingly you can already do what you want with this:

library(forcats)
x <- factor(1:4)
fct_collapse(x, A = c("1", "2"), other_level = "NULL")
#> [1] A    A    <NA> <NA>
#> Levels: A

Created on 2023-01-03 with reprex v2.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement help wanted ❤️ we'd love your help!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants