[GitHub] [arrow] nealrichardson commented on a change in pull request #7660: ARROW-9291 [R]: Support fixed size binary/list types

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

nealrichardson commented on a change in pull request #7660:
URL: https://github.com/apache/arrow/pull/7660#discussion_r451700766



##########
File path: r/src/symbols.cpp
##########
@@ -28,36 +28,41 @@ SEXP symbols::row_names = Rf_install("row.names");
 SEXP symbols::serialize_arrow_r_metadata = Rf_install(".serialize_arrow_r_metadata");
 SEXP symbols::as_list = Rf_install("as.list");
 SEXP symbols::ptype = Rf_install("ptype");
+SEXP symbols::byte_width = Rf_install("byte_width");
 
-SEXP get_classes_POSIXct() {
-  SEXP classes = Rf_allocVector(STRSXP, 2);
-  R_PreserveObject(classes);
-  SET_STRING_ELT(classes, 0, Rf_mkChar("POSIXct"));
-  SET_STRING_ELT(classes, 1, Rf_mkChar("POSIXt"));
-  return classes;
+SEXP precious(SEXP x) {

Review comment:
       Would you mind adding a comment/docstring to these new functions? It's not obvious what they do just from their names.

##########
File path: r/R/type.R
##########
@@ -301,11 +302,7 @@ large_utf8 <- function() shared_ptr(LargeUtf8, LargeUtf8__initialize())
 #' @rdname data-type
 #' @export
 binary <- function(byte_width = NULL) {

Review comment:
       Should remove the byte_width arg since it's no longer used




----------------------------------------------------------------
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