thisisnic commented on code in PR #38144:
URL: https://github.com/apache/arrow/pull/38144#discussion_r1357552848
##########
r/R/type.R:
##########
@@ -665,13 +694,16 @@ list_of <- function(type) list__(type)
LargeListType <- R6Class("LargeListType",
inherit = NestedType,
public = list(
- code = function() {
- call2("large_list_of", self$value_type$code())
+ code = function(explicit_pkg_name = FALSE) {
+ call2(private$call_name(), self$value_type$code(explicit_pkg_name), .ns
= get_pkg_ns(explicit_pkg_name))
}
),
active = list(
value_field = function() LargeListType__value_field(self),
value_type = function() LargeListType__value_type(self)
+ ),
+ private = list(
+ call_name = function() "large_list_of"
Review Comment:
The code feels more skimmable to me with the call name written directly
inside the `code()` method instead of pulled from a private field, so I'd lean
towards leaving it as-is, unless there's another strong reason to add this here?
##########
r/R/type.R:
##########
@@ -665,13 +694,16 @@ list_of <- function(type) list__(type)
LargeListType <- R6Class("LargeListType",
inherit = NestedType,
public = list(
- code = function() {
- call2("large_list_of", self$value_type$code())
+ code = function(explicit_pkg_name = FALSE) {
+ call2(private$call_name(), self$value_type$code(explicit_pkg_name), .ns
= get_pkg_ns(explicit_pkg_name))
Review Comment:
```suggestion
code = function(namespace = FALSE) {
call2(private$call_name(), self$value_type$code(explicit_pkg_name),
.ns = if(namespace) "arrow")
```
What do you think of `namespace` as a shorter-but-explicit parameter name?
Using the `.ns` parameter to `call2` is the right approach, but how about we
simplify it like this?
--
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]
