Skip to content

remove_missing() considers characters as finite #5188

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

Merged
merged 6 commits into from
Feb 21, 2023

Conversation

teunbrand
Copy link
Collaborator

This PR aims to fix #4284.

Briefly, remove_missing() now considers character vectors as finite (unless they're NA).
This makes it easier to have required aesthetics that are not numeric in custom Stats subclasses.

@teunbrand teunbrand changed the title Character is finite remove_missing() considers characters as finite Feb 14, 2023
@@ -90,7 +90,7 @@ remove_missing <- function(df, na.rm = FALSE, vars = names(df), name = "",
missing <- detect_missing(df, vars, finite)

if (any(missing)) {
df <- df[!missing, ]
df <- df[!missing, , drop = FALSE]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't occur in wild plotting because there should always be multiple columns. However, since it came up in testing, I thought to change this too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed - I think we should aim to make the code as clear and type stable as possible

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@teunbrand
Copy link
Collaborator Author

Thanks for another review Thomas!

@teunbrand teunbrand merged commit 1e1b6b1 into tidyverse:main Feb 21, 2023
@teunbrand teunbrand deleted the character_is_finite branch February 21, 2023 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove_missings(..., finite = TRUE) removes everything if there is a character column
2 participants