From 49521d079b01f9e23f2ac974ea1413d02c97196c Mon Sep 17 00:00:00 2001 From: sorhawell Date: Tue, 8 Aug 2023 12:59:49 +0200 Subject: [PATCH 1/7] prefix rust R calls to polars with polars:: --- src/rust/src/utils/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rust/src/utils/mod.rs b/src/rust/src/utils/mod.rs index 4546602ac..67b53b621 100644 --- a/src/rust/src/utils/mod.rs +++ b/src/rust/src/utils/mod.rs @@ -638,17 +638,17 @@ fn internal_rust_wrap_e(robj: Robj, str_to_lit: bool) -> RResult { match robj.rtype() { ExternalPtr if robj.inherits("Expr") => Ok(robj), ExternalPtr if robj.inherits("WhenThen") | robj.inherits("WhenThenThen") => { - unpack(R!("polars:::result({{robj}}$otherwise(pl$lit(NULL)))")) + unpack(R!("polars:::result({{robj}}$otherwise(polars::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}}))")) + unpack(R!("polars:::result(polars::pl$lit({{robj}}))")) } - _ if str_to_lit => unpack(R!("polars:::result(pl$lit({{robj}}))")), + _ if str_to_lit => unpack(R!("polars:::result(polars::pl$lit({{robj}}))")), - _ => unpack(R!("polars:::result(pl$col({{robj}}))")), + _ => unpack(R!("polars:::result(polars::pl$col({{robj}}))")), } } From 033dbad3d441812f4319a396b3be913cef0d6d99 Mon Sep 17 00:00:00 2001 From: sorhawell Date: Tue, 8 Aug 2023 13:34:00 +0200 Subject: [PATCH 2/7] fix rust R calls without polars:: --- DESCRIPTION | 3 ++- R/extendr-wrappers.R | 2 ++ src/rust/src/rlib.rs | 6 ++++++ tests/testthat/test-without_library_polars.R | 22 ++++++++++++++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 tests/testthat/test-without_library_polars.R diff --git a/DESCRIPTION b/DESCRIPTION index 1c8d61c25..b757e1a05 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -49,7 +49,8 @@ Config/Needs/dev: robinlovelace/styler.equals, spelling, stringr, - styler + styler, + callr Config/testthat/edition: 3 Collate: 'utils.R' diff --git a/R/extendr-wrappers.R b/R/extendr-wrappers.R index 8f9f45e44..8cdbd706f 100644 --- a/R/extendr-wrappers.R +++ b/R/extendr-wrappers.R @@ -67,6 +67,8 @@ 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) +test_wrong_call_pl_lit <- function(robj) .Call(wrap__test_wrong_call_pl_lit, robj) + RPolarsErr <- new.env(parent = emptyenv()) RPolarsErr$new <- function() .Call(wrap__RPolarsErr__new) diff --git a/src/rust/src/rlib.rs b/src/rust/src/rlib.rs index f999de008..1c2606fb3 100644 --- a/src/rust/src/rlib.rs +++ b/src/rust/src/rlib.rs @@ -270,6 +270,11 @@ fn test_robj_to_expr(robj: Robj) -> RResult { robj_to!(Expr, robj) } +#[extendr] +fn test_wrong_call_pl_lit(robj: Robj) -> RResult { + Ok(R!("pl$lit({{robj}})")?) // this call should have been polars::pl$lit(... +} + extendr_module! { mod rlib; fn concat_df; @@ -302,4 +307,5 @@ extendr_module! { fn test_robj_to_u32; fn test_print_string; fn test_robj_to_expr; + fn test_wrong_call_pl_lit; } diff --git a/tests/testthat/test-without_library_polars.R b/tests/testthat/test-without_library_polars.R new file mode 100644 index 000000000..3e1066a50 --- /dev/null +++ b/tests/testthat/test-without_library_polars.R @@ -0,0 +1,22 @@ +test_that("without library(polars)", { + # calling sort("mpg") triggers rust to call pl$lit() which will be available even though + # polars is not added to serach with search() library(polars) + + # positive test: + # Will work because robj_to! now calls polars::pl$lit and polars::pl$col + expect_identical( + callr::r(\() {polars::pl$DataFrame(mtcars)$sort("mpg")$to_list()}), + polars::pl$DataFrame(mtcars)$sort("mpg")$to_list() + ) + + # Negative control: + # This will fail because test_wrong_call_pl_lit just uses pl$col and pl$lit + expect_false( + callr::r(\() polars:::test_wrong_call_pl_lit(42) |> polars:::is_ok()) + ) + + # Positive-Negative control + # This works because library(polars) puts polars in search() + expect_true(polars:::test_wrong_call_pl_lit(42) |> polars:::is_ok()) + +}) From a19c24f459d9f71d3a0f65f8712de9742f17c126 Mon Sep 17 00:00:00 2001 From: sorhawell Date: Tue, 8 Aug 2023 13:34:40 +0200 Subject: [PATCH 3/7] some styling --- man/pl_corr.Rd | 2 +- man/pl_cov.Rd | 2 +- man/pl_rolling_corr.Rd | 2 +- man/pl_rolling_cov.Rd | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/man/pl_corr.Rd b/man/pl_corr.Rd index 48ca21163..8c3a011fa 100644 --- a/man/pl_corr.Rd +++ b/man/pl_corr.Rd @@ -23,6 +23,6 @@ Expr for the computed correlation Calculates the correlation between two columns } \examples{ -lf <- pl$LazyFrame(data.frame(a = c(1, 8, 3), b = c(4, 5, 2))) +lf = pl$LazyFrame(data.frame(a = c(1, 8, 3), b = c(4, 5, 2))) lf$select(pl$corr("a", "b", method = "spearman"))$collect() } diff --git a/man/pl_cov.Rd b/man/pl_cov.Rd index b7ba29486..402c7c324 100644 --- a/man/pl_cov.Rd +++ b/man/pl_cov.Rd @@ -15,7 +15,7 @@ Expr for the computed covariance Calculates the covariance between two columns / expressions. } \examples{ -lf <- pl$LazyFrame(data.frame(a = c(1, 8, 3), b = c(4, 5, 2))) +lf = pl$LazyFrame(data.frame(a = c(1, 8, 3), b = c(4, 5, 2))) lf$select(pl$cov("a", "b"))$collect() pl$cov(c(1, 8, 3), c(4, 5, 2))$to_r() } diff --git a/man/pl_rolling_corr.Rd b/man/pl_rolling_corr.Rd index 0a76e3248..79ceec9f8 100644 --- a/man/pl_rolling_corr.Rd +++ b/man/pl_rolling_corr.Rd @@ -22,6 +22,6 @@ Expr for the computed rolling correlation Calculates the rolling correlation between two columns } \examples{ -lf <- pl$LazyFrame(data.frame(a = c(1, 8, 3), b = c(4, 5, 2))) +lf = pl$LazyFrame(data.frame(a = c(1, 8, 3), b = c(4, 5, 2))) lf$select(pl$rolling_corr("a", "b", window_size = 2))$collect() } diff --git a/man/pl_rolling_cov.Rd b/man/pl_rolling_cov.Rd index ac7b5d520..98de07f68 100644 --- a/man/pl_rolling_cov.Rd +++ b/man/pl_rolling_cov.Rd @@ -22,6 +22,6 @@ Expr for the computed rolling covariance Calculates the rolling covariance between two columns } \examples{ -lf <- pl$LazyFrame(data.frame(a = c(1, 8, 3), b = c(4, 5, 2))) +lf = pl$LazyFrame(data.frame(a = c(1, 8, 3), b = c(4, 5, 2))) lf$select(pl$rolling_cov("a", "b", window_size = 2))$collect() } From b96685e7c59bf993bc94c1105e1e506a17001781 Mon Sep 17 00:00:00 2001 From: sorhawell Date: Tue, 8 Aug 2023 13:37:36 +0200 Subject: [PATCH 4/7] make fmt + news --- NEWS.md | 1 + src/rust/src/utils/mod.rs | 6 +++--- tests/testthat/test-without_library_polars.R | 7 ++++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/NEWS.md b/NEWS.md index d31b3d584..a67789baf 100644 --- a/NEWS.md +++ b/NEWS.md @@ -12,6 +12,7 @@ - New lazy function translated: `concat_str()` to concatenate several columns into one (#349). - New stat functions `pl$cov()`, `pl$rolling_cov()` `pl$corr()`, `pl$rolling_corr()` (#351). +- Fix bug to allow using polars without library(polars) (#355). # polars 0.7.0 diff --git a/src/rust/src/utils/mod.rs b/src/rust/src/utils/mod.rs index 67b53b621..2fe5ac2d0 100644 --- a/src/rust/src/utils/mod.rs +++ b/src/rust/src/utils/mod.rs @@ -637,9 +637,9 @@ fn internal_rust_wrap_e(robj: Robj, str_to_lit: bool) -> RResult { }; match robj.rtype() { ExternalPtr if robj.inherits("Expr") => Ok(robj), - ExternalPtr if robj.inherits("WhenThen") | robj.inherits("WhenThenThen") => { - unpack(R!("polars:::result({{robj}}$otherwise(polars::pl$lit(NULL)))")) - } + ExternalPtr if robj.inherits("WhenThen") | robj.inherits("WhenThenThen") => unpack(R!( + "polars:::result({{robj}}$otherwise(polars::pl$lit(NULL)))" + )), ExternalPtr if robj.inherits("When") => { rerr().plain("Cannot use a When-statement as Expr without a $then()") } diff --git a/tests/testthat/test-without_library_polars.R b/tests/testthat/test-without_library_polars.R index 3e1066a50..67c327d5b 100644 --- a/tests/testthat/test-without_library_polars.R +++ b/tests/testthat/test-without_library_polars.R @@ -5,18 +5,19 @@ test_that("without library(polars)", { # positive test: # Will work because robj_to! now calls polars::pl$lit and polars::pl$col expect_identical( - callr::r(\() {polars::pl$DataFrame(mtcars)$sort("mpg")$to_list()}), + callr::r(\() { + polars::pl$DataFrame(mtcars)$sort("mpg")$to_list() + }), polars::pl$DataFrame(mtcars)$sort("mpg")$to_list() ) # Negative control: # This will fail because test_wrong_call_pl_lit just uses pl$col and pl$lit expect_false( - callr::r(\() polars:::test_wrong_call_pl_lit(42) |> polars:::is_ok()) + callr::r(\() polars:::test_wrong_call_pl_lit(42) |> polars:::is_ok()) ) # Positive-Negative control # This works because library(polars) puts polars in search() expect_true(polars:::test_wrong_call_pl_lit(42) |> polars:::is_ok()) - }) From b258f1ea8fd376492a2efe41496ad11de2350a41 Mon Sep 17 00:00:00 2001 From: sorhawell Date: Tue, 8 Aug 2023 14:26:34 +0200 Subject: [PATCH 5/7] reorder + callr skip_if_not_installed --- DESCRIPTION | 4 ++-- tests/testthat/test-without_library_polars.R | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index b757e1a05..4c4e345d9 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -42,6 +42,7 @@ Config/Needs/website: pkgload, yaml Config/Needs/dev: + callr brio, devtools, RcppTOML, @@ -49,8 +50,7 @@ Config/Needs/dev: robinlovelace/styler.equals, spelling, stringr, - styler, - callr + styler Config/testthat/edition: 3 Collate: 'utils.R' diff --git a/tests/testthat/test-without_library_polars.R b/tests/testthat/test-without_library_polars.R index 67c327d5b..ace8876c6 100644 --- a/tests/testthat/test-without_library_polars.R +++ b/tests/testthat/test-without_library_polars.R @@ -1,7 +1,7 @@ test_that("without library(polars)", { # calling sort("mpg") triggers rust to call pl$lit() which will be available even though # polars is not added to serach with search() library(polars) - + skip_if_not_installed("callr") # positive test: # Will work because robj_to! now calls polars::pl$lit and polars::pl$col expect_identical( From efbe6312ac48bea899a4e56943dba847cbaadd7a Mon Sep 17 00:00:00 2001 From: sorhawell Date: Tue, 8 Aug 2023 14:58:59 +0200 Subject: [PATCH 6/7] missing comma in DESCRIPTION --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 4c4e345d9..a88fced15 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -42,7 +42,7 @@ Config/Needs/website: pkgload, yaml Config/Needs/dev: - callr + callr, brio, devtools, RcppTOML, From dcc3f07e94d7990cf07580b1f9a36456cce9d3c3 Mon Sep 17 00:00:00 2001 From: sorhawell Date: Tue, 8 Aug 2023 15:19:35 +0200 Subject: [PATCH 7/7] upgrade callr to suggests --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index a88fced15..16a61f59a 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -24,6 +24,7 @@ Suggests: arrow, bench, bit64, + callr, data.table, knitr, lubridate, @@ -42,7 +43,6 @@ Config/Needs/website: pkgload, yaml Config/Needs/dev: - callr, brio, devtools, RcppTOML,