nealrichardson commented on a change in pull request #10387:
URL: https://github.com/apache/arrow/pull/10387#discussion_r644135689
##########
File path: r/R/util.R
##########
@@ -110,3 +110,11 @@ handle_embedded_nul_error <- function(e) {
}
stop(e)
}
+
+is_writable_table <- function(x) {
+ inherits(x, c("data.frame", "ArrowTabular"))
+}
+
+attr(is_writable_table, "fail") <- function(call, env){
Review comment:
Can you add a comment pointing to `assertthat` and how this works?
##########
File path: r/R/util.R
##########
@@ -110,3 +110,11 @@ handle_embedded_nul_error <- function(e) {
}
stop(e)
}
+
+is_writable_table <- function(x) {
+ inherits(x, c("data.frame", "ArrowTabular"))
+}
+
+attr(is_writable_table, "fail") <- function(call, env){
+ paste0(eval(substitute(substitute(y, env), list(y = call$x)))," is not an
object of class 'data.frame', 'RecordBatch', or 'Table'.")
Review comment:
What do you think about saying "must be" instead of "is not"?
Also, previously you had said what the class of `x` was: is that feasible
here? (Ok if not or not worth it, just observing that it was nice.)
##########
File path: r/R/util.R
##########
@@ -110,3 +110,11 @@ handle_embedded_nul_error <- function(e) {
}
stop(e)
}
+
+is_writable_table <- function(x) {
+ inherits(x, c("data.frame", "ArrowTabular"))
+}
+
+attr(is_writable_table, "fail") <- function(call, env){
+ paste0(eval(substitute(substitute(y, env), list(y = call$x)))," is not an
object of class 'data.frame', 'RecordBatch', or 'Table'.")
Review comment:
Also, I don't think the eval(substitute(substitute())) is totally safe:
```
> assert_that(is_writable_table(c("a", "b")))
Error in stop(assertError(attr(res, "msg"))) : bad error message
```
`deparse(call$x)` seems to work:
```
> attr(is_writable_table, "fail") <- function(call, env){
paste0(deparse(call$x)," is not an object of class 'data.frame',
'RecordBatch', or 'Table'.")
}
> assert_that(is_writable_table(c("a", "b")))
Error: c("a", "b") is not an object of class 'data.frame', 'RecordBatch', or
'Table'.
> assert_that(is_writable_table(letters))
Error: letters is not an object of class 'data.frame', 'RecordBatch', or
'Table'.
```
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
