-
Notifications
You must be signed in to change notification settings - Fork 42
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
R floats are silently converted into Rust ints #101
Comments
Just a summary of inconsistent conversion behavior. If rust_code <- "
#[extendr]
fn slice_test(v : &[i32], i : i32) -> i32 {
v[i as usize]
}
#[extendr]
fn slice_test_na(v : &[i32], i : Option<i32>) -> Option<i32> {
match i {
Some (index) => Some(v[index as usize]),
None => None
}
}
"
rextendr::rust_source(code = rust_code)
#> build directory: C:\Users\[redacted]\AppData\Local\Temp\Rtmp2Z8D1v\filea087d24386d
# OK
slice_test(1:10, 1)
#> [1] 2
# Should not be OK
slice_test(1:10, 1.5)
#> [1] 2
# Not OK
slice_test(1:10, NA)
#> Error in slice_test(1:10, NA): unable to convert R object to primitive
# Should be OK but is not?
slice_test_na(1:10, 1)
#> Error in slice_test_na(1:10, 1): expected an integer scalar
# But this is
slice_test_na(1:10, 1L)
#> [1] 2
# This is correct
slice_test_na(1:10, 1.5)
#> Error in slice_test_na(1:10, 1.5): expected an integer scalar
# This is also correct
slice_test_na(1:10, NA)
#> [1] NA Created on 2021-01-20 by the reprex package (v0.3.0) |
Removing the "good first issue" label as I think this is a complex and somewhat tricky topic. The problem is not the implementation but the correct conceptual design. |
If a Rust function expecting an int is handed a float from R, then the float is silently converted to int, stripping any decimal values in the process. See e.g. here:
extendr/tests/extendrtests/tests/testthat/test-type-conversion.R
Line 10 in 9752c7f
I think this should instead raise an "unable to convert" error. Silent conversion from float to int is dangerous.
The text was updated successfully, but these errors were encountered: