Skip to content

Commit

Permalink
derive wrap_e from robj_to!, instead of opposite
Browse files Browse the repository at this point in the history
  • Loading branch information
sorhawell committed Aug 4, 2023
1 parent 0fc140a commit 723f488
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 16 deletions.
19 changes: 17 additions & 2 deletions R/expr__expr.R
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,16 @@ as.list.Expr = function(x, ...) {
list(x)
}

#' wrap as literal
#' DEPRECATED wrap as literal
#' @description use robj_to!(Expr) on rust side or rarely wrap_e on R-side
#' This function is only kept for reference
#' @param e an Expr(polars) or any R expression
#' @details
#' used internally to ensure an object is an expression
#' @keywords internal
#' @return Expr
#' @examples pl$col("foo") < 5
wrap_e = function(e, str_to_lit = TRUE) {
wrap_e_legacy = function(e, str_to_lit = TRUE) {
if (inherits(e, "Expr")) {
return(e)
}
Expand All @@ -96,6 +98,19 @@ wrap_e = function(e, str_to_lit = TRUE) {
}
}

#' DEPRECATED wrap as literal
#' @description use robj_to!(Expr) on rust side or rarely wrap_e on R-side
#' This function is only kept for reference
#' @param e an Expr(polars) or any R expression
#' @details
#' used internally to ensure an object is an expression
#' @keywords internal
#' @return Expr
#' @examples pl$col("foo") < 5
wrap_e = function(e, str_to_lit = TRUE) {
internal_wrap_e(e, str_to_lit) |> unwrap()
}


## TODO refactor to \(e, str_to_lit = TRUE, argname = NULL) wrap_e(e) |> result()

Expand Down
4 changes: 4 additions & 0 deletions R/extendr-wrappers.R
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ arrow_stream_to_rust <- function(rbr) invisible(.Call(wrap__arrow_stream_to_rust

dtype_str_repr <- function(dtype) .Call(wrap__dtype_str_repr, dtype)

internal_wrap_e <- function(robj, str_to_lit) .Call(wrap__internal_wrap_e, robj, str_to_lit)

test_robj_to_usize <- function(robj) .Call(wrap__test_robj_to_usize, robj)

test_robj_to_i64 <- function(robj) .Call(wrap__test_robj_to_i64, robj)
Expand All @@ -59,6 +61,8 @@ test_robj_to_u32 <- function(robj) .Call(wrap__test_robj_to_u32, robj)

test_print_string <- function(s) invisible(.Call(wrap__test_print_string, s))

test_robj_to_expr <- function(robj) .Call(wrap__test_robj_to_expr, robj)

RPolarsErr <- new.env(parent = emptyenv())

RPolarsErr$new <- function() .Call(wrap__RPolarsErr__new)
Expand Down
12 changes: 11 additions & 1 deletion R/functions__lazy.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ pl$all = function(name = NULL) {
}





#' Start Expression with a column
#' @name pl_col
#' @description
Expand Down Expand Up @@ -69,7 +72,14 @@ pl$all = function(name = NULL) {
#' # from Series of names
#' df$select(pl$col(pl$Series(c("bar", "foobar"))))
pl$col = function(name = "", ...) {
# preconvert Series into char name(s)
pl_col_internal(name, ...) |>
result() |>
unwrap("in pl$col()")
}

# This function throws error freely prefer pl$col
pl_col_internal = function(name="", ...) {
# preconvert Series into char name(s)
if (inherits(name, "Series")) name <- name$to_vector()

name_add = list(...)
Expand Down
2 changes: 1 addition & 1 deletion R/zzz.R
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ replace_private_with_pub_methods(Series, "^Series_")



# expression constructors
# expression constructors, why not just pl$lit = Expr_lit?
move_env_elements(Expr, pl, c("lit"), remove = FALSE)


Expand Down
5 changes: 3 additions & 2 deletions man/wrap_e.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions man/wrap_e_legacy.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 24 additions & 4 deletions src/rust/src/rlib.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
use crate::lazy::dsl::Expr;
use crate::rdataframe::DataFrame;
use crate::rpolarserr::{rdbg, RResult};
use crate::{rdataframe::VecDataFrame, utils::r_result_list};

use crate::lazy::dsl::ProtoExprArray;
use crate::rdataframe::DataFrame;
use crate::rdatatype::robj_to_timeunit;
use crate::robj_to;
use crate::rpolarserr::{rdbg, RResult};
use crate::series::Series;
use crate::{rdataframe::VecDataFrame, utils::r_result_list};
use extendr_api::prelude::*;
use polars::prelude as pl;
use polars_core::functions as pl_functions;
use std::result::Result;

#[extendr]
fn concat_df(vdf: &VecDataFrame) -> List {
//-> PyResult<PyDataFrame> {
Expand Down Expand Up @@ -225,6 +225,18 @@ pub fn dtype_str_repr(dtype: Robj) -> RResult<String> {
Ok(dtype.to_string())
}

// replaces wrap_e_legacy, derived from robj_to!
#[extendr]
fn internal_wrap_e(robj: Robj, str_to_lit: Robj) -> RResult<Expr> {
if robj_to!(bool, str_to_lit)? {
robj_to!(Expr, robj)
} else {
robj_to!(ExprCol, robj)
}
}

// replaces wrap_e_legacy, derived from robj_to!

// pub fn series_from_arrow(name: &str, array: Robj) -> Result<Series, String> {
// use polars::prelude::IntoSeries;
// let arr = crate::arrow_interop::to_rust::arrow_array_to_rust(array)?;
Expand Down Expand Up @@ -276,6 +288,11 @@ fn test_print_string(s: String) {
rprintln!("{}", s);
}

#[extendr]
fn test_robj_to_expr(robj: Robj) -> RResult<Expr> {
robj_to!(Expr, robj)
}

extendr_module! {
mod rlib;
fn concat_df;
Expand All @@ -298,8 +315,11 @@ extendr_module! {
fn arrow_stream_to_rust;
fn dtype_str_repr;

fn internal_wrap_e;

fn test_robj_to_usize;
fn test_robj_to_i64;
fn test_robj_to_u32;
fn test_print_string;
fn test_robj_to_expr;
}
33 changes: 27 additions & 6 deletions src/rust/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,12 +612,7 @@ pub fn robj_to_rexpr(robj: extendr_api::Robj, str_to_lit: bool) -> RResult<Expr>

//use R side wrap_e to convert any R value into Expr or
use extendr_api::*;
let robj_result_expr = R!("polars:::result(polars:::wrap_e({{robj}},{{str_to_lit}}))")
.map_err(crate::rpolarserr::extendr_to_rpolars_err)
.plain("internal error: polars:::result failed to catch this error")?;

// handle any error from wrap_e
let robj_expr = unpack_r_result_list(robj_result_expr)
let robj_expr = internal_rust_wrap_e(robj, str_to_lit)
.bad_robj(&robj_clone)
.plain("cannot be converted into an Expr")?;

Expand All @@ -631,6 +626,32 @@ pub fn robj_to_rexpr(robj: extendr_api::Robj, str_to_lit: bool) -> RResult<Expr>
Ok(Expr(ext_expr.0.clone()))
}

fn internal_rust_wrap_e(robj: Robj, str_to_lit: bool) -> RResult<Robj> {
use extendr_api::Result as EResult;
use extendr_api::Rtype::*;
use extendr_api::*;
let unpack = |res: EResult<Robj>| -> RResult<Robj> {
unpack_r_result_list(res.map_err(|err| {
extendr_api::Error::Other(format!("internal_error calling R from rust: {:?}", err))
})?)
};
match robj.rtype() {
ExternalPtr if robj.inherits("Expr") => Ok(robj),
ExternalPtr if robj.inherits("WhenThen") | robj.inherits("WhenThenThen") => {
unpack(R!("polars:::({{robj}}$otherwise(pl$lit(NULL)))"))
}
ExternalPtr if robj.inherits("When") => {
rerr().plain("Cannot use a When-statement as Expr without a $then()")
}
_h @ Logicals | _h @ List | _h @ Doubles | _h @ Integers => {
unpack(R!("polars:::result(pl$lit({{robj}}))"))
}
_ if str_to_lit => unpack(R!("polars:::result(pl$lit({{robj}}))")),

_ => unpack(R!("polars:::result(pl$col({{robj}}))")),
}
}

pub fn robj_to_lazyframe(robj: extendr_api::Robj) -> RResult<crate::rdataframe::LazyFrame> {
let robj = unpack_r_result_list(robj)?;
let rv = rdbg(&robj);
Expand Down
32 changes: 32 additions & 0 deletions tests/testthat/test-wrap_e.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#eventually these legacy behavior may drift away, then remove legacy and delete this test

test_that("wrap_e_legacy and robj_to!(Expr) produce same Expr", {
#same expression
l = list(
1, NA, pl$lit(2), 1:3, "23",
list(1:3,4:5,6L), pl$Series(3:1,"alice"), pl$lit("hej"),
pl$when(pl$col("a")=="b")$then("c"),
pl$when(pl$col("a")=="b")$then("c")$otherwise("d")
#some how NaN is not the same Expr
)
l_wrap_e = lapply(l, wrap_e_legacy)
l_robj_e = lapply(l, wrap_e)
for (i in seq_along(l)) {
e1 = l_wrap_e[[i]]
e2 = l_robj_e[[i]]
expect_true(e1$meta$eq(e2),info=capture_output_lines(print(list(i,e1,e2))))
}
})

test_that("wrap_e_legacy and robj_to!(Expr) produce same literal conversions", {

l = list(NaN) # these do produce the same conversion
l_wrap_e = lapply(l, wrap_e_legacy)
l_robj_e = lapply(l, wrap_e)
for (i in seq_along(l)) {
e1 = l_wrap_e[[i]]
e2 = l_robj_e[[i]]
expect_identical(e1$to_r(), e2$to_r())
}

})

0 comments on commit 723f488

Please sign in to comment.