Skip to content
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.

Sign up for GitHub

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

Jump to bottom

read_parquet err handling + examples #438

Closed
wants to merge 5 commits into from
Closed

read_parquet err handling + examples #438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d814235
read_parquet err handling + examples
sorhawell Oct 20, 2023
4e51310
Merge branch 'main' into read_parquet_rpolarserr
sorhawell Oct 21, 2023
d56bc0d
fix tempfile in examples
sorhawell Oct 23, 2023
777c7f2
with last
sorhawell Oct 23, 2023
5e7bf9a
update roxygen
sorhawell Oct 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions R/parquet.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,23 @@
#' @name scan_parquet
#' @rdname IO_scan_parquet
#' @examples
#' # TODO write parquet example
#' #write example file
#' my_parquet = tempfile(fileext = ".parquet")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it necessary to add processing to delete temporary files?
Also, is an extension necessary?

#' pl$LazyFrame(mtcars)$sink_parquet(my_parquet)
#'
#' # scan and get (project) only one column "cyl" but filter (apply predicate) on "hp".
#' lf = pl$scan_parquet(my_parquet)$
#' filter(pl$col("hp") > 250)$
#' select(pl$col("cyl") * 2)
#'
#' # LayFrame with a logical plan (query)
#' print(lf)
#'
#' # see optimized plan
#' lf$describe_optimized_plan()
#'
#' # Execute and get result DataFrame
#' lf$collect()
pl$scan_parquet = function(
file, # : str | Path,
n_rows = NULL, # : int | None = None,
Expand Down Expand Up @@ -64,6 +80,13 @@ pl$scan_parquet = function(
#' @param low_memory bool, try reduce memory footprint
#' @return DataFrame
#' @name read_parquet
#' @examples
#' # read parquet directly to DataFrame
#' my_parquet = tempfile(fileext = ".parquet")
#' pl$LazyFrame(mtcars)$sink_parquet(my_parquet)
#' df = pl$read_parquet(my_parquet)
#'
#' print(df)
pl$read_parquet = function(
file,
n_rows = NULL,
Expand All @@ -73,9 +96,26 @@ pl$read_parquet = function(
row_count_name = NULL,
row_count_offset = 0L,
low_memory = FALSE) {

#construct a derived call
mc = match.call()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@etiennebacher Interesting using quote + eval instead ..., I have not thought of that. What would be the pros and cons?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree scan_parquet needs a bigger overhaul. I would try to do that in separate PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This quote + eval() was already done in $read_csv(), so I just took it there but I'm not the one who implemented it. I can't think of cons for now

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eitsupi what do you think pros and cons of using ... vs match/eval ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand the intent of your question, but I think match.call is more convenient here since using ... as a function argument has the disadvantage that auto-complete cannot be used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sry my bad, I blame the fever :)

mc[[1]] = quote(pl$scan_parquet)
eval.parent(mc)$collect()

# eval call, and add to error context
mod_err_ctx = \(res) result(res) |> unwrap("in pl$read_parquet():")
lf = eval.parent(mc) |> mod_err_ctx()
lf$collect() |> mod_err_ctx()

# alternative style #1
# lf = pl$scan_parquet(...) |> mod_err_ctx()
# lf$collect() |> mod_err_ctx()

# alternative style #2
# pl$scan_parquet(...) |>
# result() |>
# and_then(\(lf) lf$collect()) |>
# unwrap("in pl$read_parquet():")

}


Expand Down
8 changes: 8 additions & 0 deletions man/IO_read_parquet.Rd
View file
Open in desktop

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

18 changes: 17 additions & 1 deletion man/IO_scan_parquet.Rd
View file
Open in desktop

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

20 changes: 20 additions & 0 deletions tests/testthat/test-parquet.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@


test_that("read_parquet", {
# throws an RPolarsRrror that attributes pl$read_parquet():
res = result(pl$read_parquet(42)) # should fail path as real is not allowed
expect_true(is_err(res))
err = res$err
expect_true(inherits(err,"RPolarsErr"))
expect_identical(res$err$get_rinfo(), "in pl$read_parquet():")
})



# # TODO! add unit tests for scan_parquet when function is refactored
# test_that("scan_parquet", {
#
#
#
#
# })