nealrichardson commented on a change in pull request #11373:
URL: https://github.com/apache/arrow/pull/11373#discussion_r727314172
##########
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:
Yeah, R supports negative `n` and does different things with it but
that's not trivial for us/I don't want to deal with it now (also this is an old
comment, just relocated in this PR)
##########
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:
You are right, it's not needed since we don't reuse this method for
Datasets etc.
##########
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:
Maybe, I saw a few places where they could intersect
##########
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:
It's not a new limitation, it's been there as long as head/tail have
been supported (version 2.0.0, one year ago), hasn't come up yet (at least on
jira, to my knowledge) 🤷
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
