jonkeane commented on a change in pull request #11373:
URL: https://github.com/apache/arrow/pull/11373#discussion_r727130827
##########
File path: r/R/dataset-scan.R
##########
@@ -143,6 +139,26 @@ Scanner$create <- function(dataset,
#' @export
names.Scanner <- function(x) names(x$schema)
+#' @export
+head.Scanner <- function(x, n = 6L, ...) {
+ assert_that(n > 0) # For now
+ dataset___Scanner__head(x, n)
+}
+
+#' @export
+tail.Scanner <- function(x, n = 6L, ...) {
+ assert_that(n > 0) # For now
Review comment:
Out of curiosity: what's the "For now" in reference to?
##########
File path: r/R/dataset-scan.R
##########
@@ -143,6 +139,26 @@ Scanner$create <- function(dataset,
#' @export
names.Scanner <- function(x) names(x$schema)
+#' @export
+head.Scanner <- function(x, n = 6L, ...) {
+ assert_that(n > 0) # For now
+ dataset___Scanner__head(x, n)
+}
+
+#' @export
+tail.Scanner <- function(x, n = 6L, ...) {
+ assert_that(n > 0) # For now
Review comment:
Ah I see now it wouldn't work with the implementation down there.
##########
File path: r/R/dplyr.R
##########
@@ -162,22 +162,35 @@ as.data.frame.arrow_dplyr_query <- function(x, row.names
= NULL, optional = FALS
#' @export
head.arrow_dplyr_query <- function(x, n = 6L, ...) {
- # TODO (ARROW-13893): refactor
- out <- head.Dataset(x, n, ...)
- restore_dplyr_features(out, x)
+ out <- as_adq(x)
+ out$head <- n
+ collapse.arrow_dplyr_query(out)
}
#' @export
tail.arrow_dplyr_query <- function(x, n = 6L, ...) {
- # TODO (ARROW-13893): refactor
- out <- tail.Dataset(x, n, ...)
- restore_dplyr_features(out, x)
+ out <- as_adq(x)
Review comment:
Do we need this `as_adq()` here? It should be harmless / a no-op, but it
should always go through the `inherits(.data, "arrow_dplyr_query")` path, no?
##########
File path: r/R/query-engine.R
##########
@@ -18,7 +18,12 @@
do_exec_plan <- function(.data) {
plan <- ExecPlan$create()
final_node <- plan$Build(.data)
- tab <- plan$Run(final_node)$read_table()
+ tab <- plan$Run(final_node)
+
+ # TODO (ARROW-14289): make the head/tail methods return RBR not Table
+ if (inherits(tab, "RecordBatchReader")) {
+ tab <- tab$read_table()
+ }
Review comment:
🤔 this might be useful in / simplify the `to_arrow()` branch
##########
File path: r/R/dataset-scan.R
##########
@@ -143,6 +139,26 @@ Scanner$create <- function(dataset,
#' @export
names.Scanner <- function(x) names(x$schema)
+#' @export
+head.Scanner <- function(x, n = 6L, ...) {
+ assert_that(n > 0) # For now
+ dataset___Scanner__head(x, n)
+}
+
+#' @export
+tail.Scanner <- function(x, n = 6L, ...) {
+ assert_that(n > 0) # For now
Review comment:
👍 on deferring this. I bet it will come up, but we can (hopefully) deal
with that when we have a proper way of doing this in a kernel.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
