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

make fifelse(test, yes, no, na) #3753

Closed
shrektan opened this issue Aug 9, 2019 · 4 comments · Fixed by #3806
Closed

make fifelse(test, yes, no, na) #3753

shrektan opened this issue Aug 9, 2019 · 4 comments · Fixed by #3806

Comments

@shrektan
Copy link
Member

@shrektan shrektan commented Aug 9, 2019

(I've comment in #3740 (comment). After a second though, I think it's better to open a new issue ticker but feel free to close it if it regards as duplicated)

Just tried the new fifelse(), fantastic!

However, I like the choice that dplyr::if_else() made - has an additional argument named missing.
It's because in R, the logic scalar may have three different values, TRUE, FALSE and NA.

Currently, fifelse() returns NA for test NA, I think it very good. But it may be better if we can have a function with an additional argument na like this:

fifelse(test, yes, no, na)

Is it a good idea?

@jangorecki
Copy link
Member

@jangorecki jangorecki commented Aug 9, 2019

I think it is good idea, and shouldn't add any overhead. Maybe only for double requires extra check for NaN.

@jangorecki jangorecki added the dev label Aug 9, 2019
@2005m
Copy link
Contributor

@2005m 2005m commented Aug 9, 2019

Yes I can suggest something and NaN are dealt with. Shall I do a pull request or shall I wait for @jangorecki PR #3740 first?

@jangorecki
Copy link
Member

@jangorecki jangorecki commented Aug 9, 2019

Generally it would be fine to push it to the same PR branch so we can deal with conflicts easily, but I feel we might need to revert some of my chances there, so probably better to wait for @mattdowle to review #3741 first.

@2005m
Copy link
Contributor

@2005m 2005m commented Aug 31, 2019

Hello, just saw that #3741 has been reviewed. Where would you like me to commit changes for feature request #3753 ? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants