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

fifelse() coerces NA to other types and supports vectorized na argument #4289

Merged
merged 9 commits into from
May 14, 2021

Conversation

shrektan
Copy link
Member

@shrektan shrektan commented Mar 7, 2020

Closes #4287
Closes #4286
Closes #4277

Two points:

  1. na supports vectorized input: The na argument should be no differences from yes and no due to the fact that TRUE, FALSE and NA are all valid R's logical values
  2. The logical value of NAwill be coerced to other types automatically:
    1. The na argument already performs this behavior. As stated in the 1 point, yes and no should behave the same;
    2. data.table does this auto-coercion in many other functions
    3. Should only have a very small performance impact

inst/tests/tests.Rraw Outdated Show resolved Hide resolved
src/fifelse.c Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 7, 2020

Codecov Report

Merging #4289 (0c810d2) into master (9847445) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4289   +/-   ##
=======================================
  Coverage   99.47%   99.47%           
=======================================
  Files          75       75           
  Lines       14726    14775   +49     
=======================================
+ Hits        14648    14697   +49     
  Misses         78       78           
Impacted Files Coverage Δ
src/fifelse.c 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9847445...0c810d2. Read the comment docs.

@mattdowle mattdowle added this to the 1.14.1 milestone May 14, 2021
src/fifelse.c Outdated Show resolved Hide resolved
@mattdowle mattdowle added the WIP label May 14, 2021
@mattdowle mattdowle removed the WIP label May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants