Skip to content

Commit

Permalink
Remove TryFrom<Sexp> for usize and add NumericScalar::as_usize(),…
Browse files Browse the repository at this point in the history
… `NumericSexp::iter_usize()` (#311)
  • Loading branch information
yutannihilation authored Oct 20, 2024
1 parent 7c36f4f commit 3cfb6f6
Show file tree
Hide file tree
Showing 14 changed files with 229 additions and 46 deletions.
46 changes: 46 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,52 @@
<!-- next-header -->
## [Unreleased] (ReleaseDate)

### Breaking Change

Removed `TryFrom<Sexp> for usize`, so the following code no longer compiles.

```rust
#[savvy]
fn foo(x: usize) -> savvy::Result<()> {
...
}
```

Instead, you can use `i32` and convert it to `usize` by yourself. If you are
sure the input number is never negative, you can just use the `as` conversion.
If you are not sure, you should use `<usize>::try_from()` and handle the error
by yourself. Also, please be aware you need to handle NA as well.

```rust
#[savvy]
fn foo(x: i32) -> savvy::Result<()> {
if x.is_na() {
return Err("cannot convert NA to usize".into())?;
}

let x = <usize>::try_from(x).map_err(|e| e.to_string().into());

...
}
```

Alternatively, you can use newly-added methods, `NumericScalar::as_usize()` and
`NumericSexp::iter_usize()`. What's good is that this can handle integer-ish
numeric, which means you can allow users to input a larger number than the
integer max (2147483647)!

```rust
fn usize_to_string_scalar(x: NumericScalar) -> savvy::Result<Sexp> {
let x_usize = x.as_usize()?;
x_usize.to_string().try_into()
}
```

```r
usize_to_string_scalar(2147483648)
#> [1] "2147483648"
```

## [v0.6.8] (2024-09-17)

### Minor Improvements
Expand Down
15 changes: 10 additions & 5 deletions R-package/R/000-wrappers.R
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,6 @@ NULL
}


`scalar_input_usize` <- function(`x`) {
invisible(.Call(savvy_scalar_input_usize__impl, `x`))
}


`scalar_input_real` <- function(`x`) {
invisible(.Call(savvy_scalar_input_real__impl, `x`))
}
Expand Down Expand Up @@ -554,6 +549,11 @@ NULL
}


`usize_to_string` <- function(`x`) {
.Call(savvy_usize_to_string__impl, `x`)
}


`times_two_numeric_f64_scalar` <- function(`x`) {
.Call(savvy_times_two_numeric_f64_scalar__impl, `x`)
}
Expand All @@ -564,6 +564,11 @@ NULL
}


`usize_to_string_scalar` <- function(`x`) {
.Call(savvy_usize_to_string_scalar__impl, `x`)
}


`print_numeric` <- function(`x`) {
invisible(.Call(savvy_print_numeric__impl, `x`))
}
Expand Down
18 changes: 12 additions & 6 deletions R-package/src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -304,11 +304,6 @@ SEXP savvy_scalar_input_int__impl(SEXP c_arg__x) {
return handle_result(res);
}

SEXP savvy_scalar_input_usize__impl(SEXP c_arg__x) {
SEXP res = savvy_scalar_input_usize__ffi(c_arg__x);
return handle_result(res);
}

SEXP savvy_scalar_input_real__impl(SEXP c_arg__x) {
SEXP res = savvy_scalar_input_real__ffi(c_arg__x);
return handle_result(res);
Expand Down Expand Up @@ -524,6 +519,11 @@ SEXP savvy_times_two_numeric_i32__impl(SEXP c_arg__x) {
return handle_result(res);
}

SEXP savvy_usize_to_string__impl(SEXP c_arg__x) {
SEXP res = savvy_usize_to_string__ffi(c_arg__x);
return handle_result(res);
}

SEXP savvy_times_two_numeric_f64_scalar__impl(SEXP c_arg__x) {
SEXP res = savvy_times_two_numeric_f64_scalar__ffi(c_arg__x);
return handle_result(res);
Expand All @@ -534,6 +534,11 @@ SEXP savvy_times_two_numeric_i32_scalar__impl(SEXP c_arg__x) {
return handle_result(res);
}

SEXP savvy_usize_to_string_scalar__impl(SEXP c_arg__x) {
SEXP res = savvy_usize_to_string_scalar__ffi(c_arg__x);
return handle_result(res);
}

SEXP savvy_print_numeric__impl(SEXP c_arg__x) {
SEXP res = savvy_print_numeric__ffi(c_arg__x);
return handle_result(res);
Expand Down Expand Up @@ -770,7 +775,6 @@ static const R_CallMethodDef CallEntries[] = {
{"savvy_abs_complex__impl", (DL_FUNC) &savvy_abs_complex__impl, 1},
{"savvy_new_value_pair__impl", (DL_FUNC) &savvy_new_value_pair__impl, 2},
{"savvy_scalar_input_int__impl", (DL_FUNC) &savvy_scalar_input_int__impl, 1},
{"savvy_scalar_input_usize__impl", (DL_FUNC) &savvy_scalar_input_usize__impl, 1},
{"savvy_scalar_input_real__impl", (DL_FUNC) &savvy_scalar_input_real__impl, 1},
{"savvy_scalar_input_logical__impl", (DL_FUNC) &savvy_scalar_input_logical__impl, 1},
{"savvy_scalar_input_string__impl", (DL_FUNC) &savvy_scalar_input_string__impl, 1},
Expand Down Expand Up @@ -813,8 +817,10 @@ static const R_CallMethodDef CallEntries[] = {
{"savvy_new_bool__impl", (DL_FUNC) &savvy_new_bool__impl, 1},
{"savvy_times_two_numeric_f64__impl", (DL_FUNC) &savvy_times_two_numeric_f64__impl, 1},
{"savvy_times_two_numeric_i32__impl", (DL_FUNC) &savvy_times_two_numeric_i32__impl, 1},
{"savvy_usize_to_string__impl", (DL_FUNC) &savvy_usize_to_string__impl, 1},
{"savvy_times_two_numeric_f64_scalar__impl", (DL_FUNC) &savvy_times_two_numeric_f64_scalar__impl, 1},
{"savvy_times_two_numeric_i32_scalar__impl", (DL_FUNC) &savvy_times_two_numeric_i32_scalar__impl, 1},
{"savvy_usize_to_string_scalar__impl", (DL_FUNC) &savvy_usize_to_string_scalar__impl, 1},
{"savvy_print_numeric__impl", (DL_FUNC) &savvy_print_numeric__impl, 1},
{"savvy_default_value_scalar__impl", (DL_FUNC) &savvy_default_value_scalar__impl, 1},
{"savvy_default_value_vec__impl", (DL_FUNC) &savvy_default_value_vec__impl, 1},
Expand Down
3 changes: 2 additions & 1 deletion R-package/src/rust/api.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ SEXP savvy_first_complex__ffi(SEXP c_arg__x);
SEXP savvy_abs_complex__ffi(SEXP c_arg__x);
SEXP savvy_new_value_pair__ffi(SEXP c_arg__a, SEXP c_arg__b);
SEXP savvy_scalar_input_int__ffi(SEXP c_arg__x);
SEXP savvy_scalar_input_usize__ffi(SEXP c_arg__x);
SEXP savvy_scalar_input_real__ffi(SEXP c_arg__x);
SEXP savvy_scalar_input_logical__ffi(SEXP c_arg__x);
SEXP savvy_scalar_input_string__ffi(SEXP c_arg__x);
Expand Down Expand Up @@ -96,8 +95,10 @@ SEXP savvy_new_real__ffi(SEXP c_arg__size);
SEXP savvy_new_bool__ffi(SEXP c_arg__size);
SEXP savvy_times_two_numeric_f64__ffi(SEXP c_arg__x);
SEXP savvy_times_two_numeric_i32__ffi(SEXP c_arg__x);
SEXP savvy_usize_to_string__ffi(SEXP c_arg__x);
SEXP savvy_times_two_numeric_f64_scalar__ffi(SEXP c_arg__x);
SEXP savvy_times_two_numeric_i32_scalar__ffi(SEXP c_arg__x);
SEXP savvy_usize_to_string_scalar__ffi(SEXP c_arg__x);
SEXP savvy_print_numeric__ffi(SEXP c_arg__x);
SEXP savvy_default_value_scalar__ffi(SEXP c_arg__x);
SEXP savvy_default_value_vec__ffi(SEXP c_arg__x);
Expand Down
4 changes: 2 additions & 2 deletions R-package/src/rust/src/complex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use savvy::savvy;
use savvy::NotAvailableValue;

#[savvy]
fn new_complex(size: usize) -> savvy::Result<savvy::Sexp> {
savvy::OwnedComplexSexp::new(size)?.into()
fn new_complex(size: i32) -> savvy::Result<savvy::Sexp> {
savvy::OwnedComplexSexp::new(size as usize)?.into()
}

#[savvy]
Expand Down
6 changes: 0 additions & 6 deletions R-package/src/rust/src/convert_from_rust_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,6 @@ fn scalar_input_int(x: i32) -> savvy::Result<()> {
Ok(())
}

#[savvy]
fn scalar_input_usize(x: usize) -> savvy::Result<()> {
savvy::r_println!("{x}");
Ok(())
}

#[savvy]
fn scalar_input_real(x: f64) -> savvy::Result<()> {
savvy::r_println!("{x}");
Expand Down
12 changes: 6 additions & 6 deletions R-package/src/rust/src/init_vectors.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
use savvy::savvy;

#[savvy]
fn new_int(size: usize) -> savvy::Result<savvy::Sexp> {
savvy::OwnedIntegerSexp::new(size)?.into()
fn new_int(size: i32) -> savvy::Result<savvy::Sexp> {
savvy::OwnedIntegerSexp::new(size as usize)?.into()
}

#[savvy]
fn new_real(size: usize) -> savvy::Result<savvy::Sexp> {
savvy::OwnedRealSexp::new(size)?.into()
fn new_real(size: i32) -> savvy::Result<savvy::Sexp> {
savvy::OwnedRealSexp::new(size as usize)?.into()
}

#[savvy]
fn new_bool(size: usize) -> savvy::Result<savvy::Sexp> {
savvy::OwnedLogicalSexp::new(size)?.into()
fn new_bool(size: i32) -> savvy::Result<savvy::Sexp> {
savvy::OwnedLogicalSexp::new(size as usize)?.into()
}
17 changes: 16 additions & 1 deletion R-package/src/rust/src/numeric.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use savvy::{
r_println, savvy, NotAvailableValue, NumericScalar, NumericSexp, NumericTypedSexp,
OwnedIntegerSexp, OwnedRealSexp, Sexp,
OwnedIntegerSexp, OwnedRealSexp, OwnedStringSexp, Sexp,
};

#[savvy]
Expand Down Expand Up @@ -34,6 +34,16 @@ fn times_two_numeric_i32(x: NumericSexp) -> savvy::Result<Sexp> {
out.into()
}

#[savvy]
fn usize_to_string(x: NumericSexp) -> savvy::Result<Sexp> {
let mut out = OwnedStringSexp::new(x.len())?;
for (i, v) in x.iter_usize().enumerate() {
out.set_elt(i, &v?.to_string())?;
}

out.into()
}

#[savvy]
fn times_two_numeric_f64_scalar(x: NumericScalar) -> savvy::Result<Sexp> {
let v = x.as_f64();
Expand All @@ -54,6 +64,11 @@ fn times_two_numeric_i32_scalar(x: NumericScalar) -> savvy::Result<Sexp> {
}
}

#[savvy]
fn usize_to_string_scalar(x: NumericScalar) -> savvy::Result<Sexp> {
x.as_usize()?.to_string().try_into()
}

#[savvy]
fn print_numeric(x: NumericSexp) -> savvy::Result<()> {
match x.into_typed() {
Expand Down
3 changes: 0 additions & 3 deletions R-package/tests/testthat/test-from-rust-types.R
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
test_that("scalar functions reject non-scalar values and missing values", {
# no error
expect_output(scalar_input_int(1L), "1")
expect_output(scalar_input_usize(1L), "1")
expect_output(scalar_input_usize(0L), "0")
expect_output(scalar_input_real(1.3), "1.3")
expect_output(scalar_input_logical(FALSE), "false")
expect_output(scalar_input_string("foo"), "foo")

# error
expect_error(scalar_input_int(1:10))
expect_error(scalar_input_usize(-1L))
expect_error(scalar_input_real(c(1, 2)))
expect_error(scalar_input_logical(c(TRUE, FALSE)))
expect_error(scalar_input_str(c("foo", "bar")))
Expand Down
44 changes: 44 additions & 0 deletions R-package/tests/testthat/test-numeric.R
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,30 @@ test_that("NumericSexp works", {
expect_error(times_two_numeric_i32(c(1.1, -1.1))) # not integer-ish
})

test_that("NumericSexp works for usize conversions", {
# i32 to usize
expect_equal(
usize_to_string(c(0L, 10L)),
c("0", "10")
)

# f64 to usize
expect_equal(
# 2147483647 = .Machine$integer.max
usize_to_string(c(0.0, 10.0, 2147483648.0)),
c("0", "10", "2147483648")
)

# error cases
expect_error(usize_to_string(NA_integer_))
expect_error(usize_to_string(NA_real_))
expect_error(usize_to_string(Inf))
expect_error(usize_to_string(NaN))
expect_error(usize_to_string(-1L))
expect_error(usize_to_string(-1.0))
})


test_that("NumericScalar works", {
expect_equal(times_two_numeric_f64_scalar(1L), 2)
expect_equal(times_two_numeric_f64_scalar(1), 2)
Expand All @@ -46,3 +70,23 @@ test_that("NumericScalar works", {
expect_error(times_two_numeric_i32_scalar(2147483648)) # out of i32's range
expect_error(times_two_numeric_i32_scalar(1.1)) # not integer-ish
})

test_that("NumericScalar works for usize conversions", {
# i32 to usize
expect_equal(usize_to_string_scalar(0L), "0")
expect_equal(usize_to_string_scalar(10L), "10")

# f64 to usize
expect_equal(usize_to_string_scalar(0.0), "0")
expect_equal(usize_to_string_scalar(10.0), "10")
# 2147483647 = .Machine$integer.max
expect_equal(usize_to_string_scalar(2147483648.0), "2147483648")

# error cases
expect_error(usize_to_string_scalar(NA_integer_))
expect_error(usize_to_string_scalar(NA_real_))
expect_error(usize_to_string_scalar(Inf))
expect_error(usize_to_string_scalar(NaN))
expect_error(usize_to_string_scalar(-1L))
expect_error(usize_to_string_scalar(-1.0))
})
7 changes: 5 additions & 2 deletions book/src/atomic_types.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ it fails when any of the values is
- out of range for `i32`
- not integer-ish (e.g. `1.1`)

For example, you can rewrite the above function like this:
For convenience, `NumericSexp` also provides `iter_usize()`, which returns an
iterator of `Result<usize>`.

With `NumericSexp`, you can rewrite the above `times_two` function like this:

```rust
#[savvy]
Expand All @@ -104,7 +107,7 @@ fn times_two(x: NumericSexp) -> savvy::Result<Sexp> {
}
```

Another way is to use `.into_typed()` and `match` the result to apply an
Alternatively, you can use `.into_typed()` and `match` the result to apply an
appropriate function depneding on the type. In this case, you need to define two
different functions, but this might be useful when the logic is very different
for integer values and real values.
Expand Down
19 changes: 18 additions & 1 deletion book/src/scalar.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,30 @@ fn times_two_numeric_i32_scalar(x: NumericScalar) -> savvy::Result<Sexp> {
}
```

Note that, while `as_f64()` is infallible, `as_i32()` can fail when the
Note that, while `as_f64()` is infallible, `as_i32()` can fail when the
conversion is from `f64` to `i32` and

- the value is `Inf` or `-Inf`
- the value is out of range for `i32`
- the value is not integer-ish (e.g. `1.1`)

For convenience, `NumericScalar` also provides a conversion to usize by
`as_usize()`. What's good is that this can handle integer-ish numeric, which
means you can allow users to input a larger number than the integer max
(2147483647)!

```rust
fn usize_to_string_scalar(x: NumericScalar) -> savvy::Result<Sexp> {
let x_usize = x.as_usize()?;
x_usize.to_string().try_into()
}
```

```r
usize_to_string_scalar(2147483648)
#> [1] "2147483648"
```

## Output

Just like a Rust vector, a Rust scalar value can be converted into `Sexp` by
Expand Down
Loading

0 comments on commit 3cfb6f6

Please sign in to comment.