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

dealing with NAs #39

Open
moodymudskipper opened this issue Nov 20, 2019 · 6 comments
Open

dealing with NAs #39

moodymudskipper opened this issue Nov 20, 2019 · 6 comments

Comments

@moodymudskipper
Copy link
Owner

@KKPMW Our package functions produce NAs, which might not always be desirable for all situations:

c(1:4) %in{}% c(4,NA,3)
#> [1]   NA   NA TRUE TRUE
c(1:4) %[in{}% c(4,NA,3)
#> [1] NA NA  3  4

do you think it would stay in the scope of the package to add helpers to remove NAs or replace them by FALSE (or by symetry though less useful, by TRUE)

c(1:4) %in{}% na_rm(c(4,NA,3))
#> [1] FALSE FALSE TRUE TRUE

c(1:4) %[in{}% na_rm(c(4,NA,3))
#> [1] 3 4

na_false(c(1:4) %in{}% c(4,NA,3))
#> [1] FALSE FALSE TRUE TRUE
@karoliskoncevicius
Copy link
Collaborator

hmm to me it looks a bit too complicated.

If NAs are not desirable we can actually switch that particular behaviour to match %in% instead of ==. But then the inconsistency with [== will still remain.

Another alternative is to provide slightly different operator syntax for dealing with NAs differently. Maybe capital letters %IN{}%, like a "strict" %in{}% But not sure if it's intuitive.

@moodymudskipper
Copy link
Owner Author

I think I'd rather not multiply by two our whole set of operators, I miss these functions in a CRAN package, but this might not be the place for them.

@karoliskoncevicius
Copy link
Collaborator

Yup, agreed. What is your opinion about the other option - breaking consistency with == just for NA values?

@moodymudskipper
Copy link
Owner Author

I'm not sure about it, it's easy to remove NAs or transform them, if they're gone by default you won't get them back.

unrelated FYI : rstudio/rstudio#5767

@moodymudskipper
Copy link
Owner Author

I don't completely dislike the capital letter idea though! We can think about it as we use the package.

@karoliskoncevicius
Copy link
Collaborator

karoliskoncevicius commented Nov 20, 2019

Yup I agree about not getting NA values back if we get rid of it. Very good point.

About rstudio issue - great catch. I do not use R-studio, so do not have this particular problem. But it would be cumbersome for potential users of the package.

@moodymudskipper moodymudskipper changed the title na_rm, na_false, na_true dealing with NAs Nov 25, 2019
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

No branches or pull requests

2 participants