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_reorder() treatment of missing values in .x depends on whether .f is a factor or a character vector #359

Open
dominikbach opened this issue Nov 17, 2023 · 1 comment
Labels
bug an unexpected problem or unintended behavior

Comments

@dominikbach
Copy link

dominikbach commented Nov 17, 2023

When .x contains missing values, this throws an error if .f is a character vector but not if .f is a factor (with no missing values in either case).

This behaviour is not documented (and I suspect is not intended). It occurs in v0.5.0 and v1.0.0.

Example:

.f <- c("a", "b", "c")
.x <- c(1, 2, NA)
forcats::fct_reorder(.f, .x)
forcats::fct_reorder(forcats::as_factor(.f), .x)
@dominikbach dominikbach changed the title fct_reorder treatment of missing values in .x depends on whether .f is a factor is a character vector fct_reorder treatment of missing values in .x depends on whether .f is a factor or a character vector Nov 17, 2023
@dominikbach dominikbach changed the title fct_reorder treatment of missing values in .x depends on whether .f is a factor or a character vector fct_reorder treatment of missing values in .x depends on whether .f is a factor or a character vector Nov 17, 2023
@hadley
Copy link
Member

hadley commented Oct 21, 2024

library(forcats)

.f <- c("a", "b", "c")
.x <- c(1, 2, NA)
fct_reorder(.f, .x)
#> Warning: `fct_reorder()` removing 1 missing value.
#> ℹ Use `.na_rm = TRUE` to silence this message.
#> ℹ Use `.na_rm = FALSE` to preserve NAs.
#> Error in `lvls_reorder()`:
#> ! `idx` must contain one integer for each level of `f`
fct_reorder(as_factor(.f), .x)
#> Warning: `fct_reorder()` removing 1 missing value.
#> ℹ Use `.na_rm = TRUE` to silence this message.
#> ℹ Use `.na_rm = FALSE` to preserve NAs.
#> [1] a b c
#> Levels: a b c

Created on 2024-10-21 with reprex v2.1.0

Probably check_factor() should use as_factor()

@hadley hadley added the bug an unexpected problem or unintended behavior label Oct 21, 2024
@hadley hadley changed the title fct_reorder treatment of missing values in .x depends on whether .f is a factor or a character vector fct_reorder() treatment of missing values in .x depends on whether .f is a factor or a character vector Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

2 participants