thisisnic commented on code in PR #13160:
URL: https://github.com/apache/arrow/pull/13160#discussion_r873660800
##########
r/R/dplyr-funcs.R:
##########
@@ -58,15 +58,16 @@ NULL
#' @keywords internal
#'
register_binding <- function(fun_name, fun, registry = nse_funcs) {
- name <- gsub("^.*?::", "", fun_name)
+ name <- gsub("^.*?_", "", fun_name)
namespace <- gsub("::.*$", "", fun_name)
Review Comment:
I haven't thought about this in great detail, but I'd be expecting an
implementation of this to use the `namespace` variable here, or remove it.
I'm just wondering if there might be some advantages to recording the
function and namespace separately instead of appending them? The former feels
"cleaner" in my head, though that's not sufficient reason to suggest changing
it.
A thought; how do we distinguish between different kinds of functions? i.e.
we have:
* bindings for functions from packages, e.g. `lubridate::as_datetime()`
* bindings directly to arrow compute functions, e.g.
`arrow_utf8_split_whitespace()`
* other bindings list the one for `cast()`
In the last case here, if we have other functions which have underscores but
no namespace, I think the regex would no longer be extracting the right name.
--
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]
