[GitHub] [arrow] jonkeane commented on a change in pull request #10343: ARROW-12758: [R] Add examples to more function documentation

[GitBox <gi...@apache.org>]

jonkeane commented on a change in pull request #10343:
URL: https://github.com/apache/arrow/pull/10343#discussion_r634453920



##########
File path: r/R/buffer.R
##########
@@ -25,13 +25,19 @@
 #' `buffer()` lets you create an `arrow::Buffer` from an R object
 #' @section Methods:
 #'
-#' - `$is_mutable()` :
-#' - `$ZeroPadding()` :
-#' - `$size()` :
-#' - `$capacity()`:
+#' - `$is_mutable` : is this buffer mutable?
+#' - `$ZeroPadding()` : zero bytes in padding, i.e. bytes between size and capacity
+#' - `$size` : size in memory, in bytes
+#' - `$capacity`: possible capacity, in bytes 

Review comment:
       Thank you for catching all of these that had `()` and shouldn't have + adding in the explanations 

##########
File path: r/R/array.R
##########
@@ -84,6 +84,28 @@
 #'
 #' @rdname array
 #' @name array
+#' @examples
+#' vals <- 1:10
+#' my_array <- Array$create(vals)
+#' my_array$type
+#' my_array$cast(int8())
+#' 
+#' # Check if value is null; zero-indexed
+#' na_array <- Array$create(c(1:5, NA))
+#' na_array$IsNull(0)
+#' na_array$IsNull(5)
+#' na_array$IsValid(5)
+#' na_array$null_count
+#' 
+#' # zero-copy slicing; the offset of the new Array will be the same as the index passed to $Slice

Review comment:
       This is probably (definitely) not the right place for it, and feel free to punt on this, but it would be nice to have a description of why one might want zero-copy and what the benefits of it are.

##########
File path: r/R/array.R
##########
@@ -84,6 +84,28 @@
 #'
 #' @rdname array
 #' @name array
+#' @examples
+#' vals <- 1:10

Review comment:
       Is `vals` here reused? I don't see it, but might be missing it in code folding... If not, it might be clearer to use `1:10` in the line below

##########
File path: r/R/compute.R
##########
@@ -229,6 +233,25 @@ all.ArrowDatum <- function(..., na.rm = FALSE){
 #' as `x` with the (0-based) indexes into `table`. `is_in()` returns a
 #' `boolean`-type `Array` of the same length as `x` with values indicating
 #' per element of `x` it it is present in `table`.
+#' @examples
+#' # note that the returned value is 0-indexed
+#' cars_tbl <- Table$create(name = rownames(mtcars), mtcars)
+#' match_arrow(Array$create("Mazda RX4 Wag"), cars_tbl$name)

Review comment:
       Would it be helpful to include something like `match_arrow(Scalar$create("Mazda RX4 Wag"), cars_tbl$name)` as well to show that one can use a scalar or an array? It looks like the docs above don't actually mention this, so maybe it's an off-label use that we should avoid? Or it's been added and we should document it...

##########
File path: r/R/compute.R
##########
@@ -264,6 +287,9 @@ is_in.ArrowDatum <- function(x, table, ...) {
 #' @param x `Array` or `ChunkedArray`
 #' @return A `StructArray` containing "values" (same type as `x`) and "counts"
 #' `Int64`.
+#' @examples
+#' cyl_vals <- Array$create(mtcars$cyl)
+#' value_counts(cyl_vals)

Review comment:
       This is less a concern of the examples, and more the code for the `StructArray` print method (I think): I'm a little surprised that the names `value` and `counts` aren't displayed:
   
   ```
   > value_counts(Array$create(mtcars$cyl))
   StructArray
   <struct<values: double, counts: int64>>
   -- is_valid: all not null
   -- child 0 type: double
     [
       6,
       4,
       8
     ]
   -- child 1 type: int64
     [
       7,
       11,
       14
     ]
   ```




-- 
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:
users@infra.apache.org