Skip to content

Commit

Permalink
fix: nested null data type Series treated as an empty Series (#1023)
Browse files Browse the repository at this point in the history
  • Loading branch information
eitsupi authored Apr 11, 2024
1 parent 5aa0b50 commit de1fd05
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 12 deletions.
2 changes: 1 addition & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
### Other breaking changes

- R objects inside an R list are now converted to Polars data types via
`as_polars_series()` (#1021, #1022). For example, up to polars 0.15.1,
`as_polars_series()` (#1021, #1022, #1023). For example, up to polars 0.15.1,
a list containing a data.frame with a column of `{clock}` naive-time class
was converted to a nested List type of Float64:

Expand Down
6 changes: 4 additions & 2 deletions R/as_polars.R
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,12 @@ as_polars_lf.RPolarsLazyGroupBy = function(x, ...) {
#'
#' as_polars_series(data.frame(a = 1:4))
#'
#' as_polars_series(pl$Series(1:4, name = "foo"))
#' as_polars_series(as_polars_series(1:4, name = "foo"))
#'
#' as_polars_series(pl$lit(1:4))
#'
#' # Nested type support
#' as_polars_series(list(data.frame(a = I(list(1:4)))))
as_polars_series = function(x, name = NULL, ...) {
UseMethod("as_polars_series")
}
Expand Down Expand Up @@ -525,7 +528,6 @@ as_polars_series.clock_zoned_time = function(x, name = NULL, ...) {
}


# TODO: rewrite `recursive_robjname2series_tree` in Rust side
#' @rdname as_polars_series
#' @export
as_polars_series.list = function(x, name = NULL, ...) {
Expand Down
5 changes: 4 additions & 1 deletion man/as_polars_series.Rd

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

11 changes: 8 additions & 3 deletions src/rust/src/conversion_r_to_s.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,18 @@ fn recursive_robjname2series_tree(x: &Robj, name: &str) -> pl::PolarsResult<Seri

// handle any supported Robj
let series_result = match rtype {
Rtype::Null => Ok(SeriesTree::SeriesEmptyVec), // flag NULL with this enum, to resolve polars type later

Rtype::ExternalPtr => match () {
_ if x.inherits("RPolarsSeries") => {
let s: RPolarsSeries =
unsafe { &mut *x.external_ptr_addr::<RPolarsSeries>() }.clone();
Ok(SeriesTree::Series(s.0))

let dtype = s.0.dtype().clone();
match dtype {
pl::DataType::Null => Ok(SeriesTree::SeriesEmptyVec),
_ => Ok(SeriesTree::Series(s.0)),
}
}
_ => Err(pl::PolarsError::InvalidOperation(
"This externalPtr is currently not supported".into(),
Expand Down Expand Up @@ -145,8 +152,6 @@ fn recursive_robjname2series_tree(x: &Robj, name: &str) -> pl::PolarsResult<Seri
Ok(SeriesTree::Series(s))
}

Rtype::Null => Ok(SeriesTree::SeriesEmptyVec), // flag NULL with this enum, to resolve polars type later

Rtype::Raw => {
let rpolars_raw_list = list!(x)
.set_class(["rpolars_raw_list", "list"])
Expand Down
17 changes: 12 additions & 5 deletions tests/testthat/test-as_polars.R
Original file line number Diff line number Diff line change
Expand Up @@ -438,10 +438,17 @@ test_that("as_polars_series for nested type", {
list(data.frame(a = I(list(data.frame(b = 1L)))))
)$dtype == pl$List(pl$Struct(a = pl$List(pl$Struct(b = pl$Int32))))
)

# TODO: this shouldn't error
expect_grepl_error(
as_polars_series(list(as_polars_series(NULL), as_polars_series(1L))),
"One element was null and another was i32"
expect_true(
as_polars_series(list(as_polars_series(NULL), as_polars_series(1L)))$dtype == pl$List(pl$Int32)
)
expect_true(
as_polars_series(list(NULL, list(1, 2)))$equals(
as_polars_series(list(list(), list(1, 2)))
)
)
expect_true(
as_polars_series(list(as_polars_series(NULL), list(1, 2)))$equals(
as_polars_series(list(list(), list(1, 2)))
)
)
})

0 comments on commit de1fd05

Please sign in to comment.