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

Should nafill replace NaN values? #4020

Closed
ToeKneeFan opened this issue Nov 1, 2019 · 1 comment · Fixed by #4025
Closed

Should nafill replace NaN values? #4020

ToeKneeFan opened this issue Nov 1, 2019 · 1 comment · Fixed by #4025

Comments

@ToeKneeFan
Copy link

@ToeKneeFan ToeKneeFan commented Nov 1, 2019

This is more of a question than a feature request. The majority of answers to the SO question "Fastest way to replace NAs in a large data.table" use approaches that replace both NA and NaN, since is.na(NaN) is true. However, nafill and setnafill at the moment do not replace NaN values. Should these new functions be expected to fill NaN values as well?

# Minimal reproducible example

library(data.table)

x <- c(NA, NaN, 0, 1, 2)
y <- x

# using is.na to replace
y[is.na(y)] <- 0
print(y)
# [1] 0 0 0 1 2

# using nafill to replace
nafill(x, fill = 0)
# [1]   0 NaN   0   1   2

# Output of sessionInfo()

R version 3.6.0 (2019-04-26)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: macOS Mojave 10.14.6

Matrix products: default
BLAS:   /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/3.6/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] data.table_1.12.6

loaded via a namespace (and not attached):
[1] compiler_3.6.0 tools_3.6.0    knitr_1.25     xfun_0.10  
@jangorecki
Copy link
Member

@jangorecki jangorecki commented Nov 2, 2019

I would say there should be an option, NaN is logically distinct from NA.

@jangorecki jangorecki added this to the 1.12.7 milestone Nov 29, 2019
@mattdowle mattdowle removed this from the 1.12.7 milestone Dec 8, 2019
@mattdowle mattdowle added this to the 1.12.9 milestone Dec 8, 2019
@jangorecki jangorecki removed this from the 1.12.11 milestone May 26, 2020
@jangorecki jangorecki added this to the 1.12.9 milestone May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants