You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "DavisVaughan (via GitHub)" <gi...@apache.org> on 2023/02/27 15:59:01 UTC
[GitHub] [arrow] DavisVaughan opened a new issue, #34366: Don't `getFromNamespace()` the `dplyr:::check_name()` helper
DavisVaughan opened a new issue, #34366:
URL: https://github.com/apache/arrow/issues/34366
### Describe the bug, including details regarding any error messages, version, and platform.
Right here arrow grabs the `dplyr:::check_name()` helper:
https://github.com/apache/arrow/blob/969dd290ab93c8629b643134f150e71c1c05c2ff/r/R/dplyr-count.R#L42-L43
We've renamed this in dplyr 1.1.1 to `check_n_name()` as part of https://github.com/tidyverse/dplyr/pull/6722
That PR is part of a larger move towards using some standard internal `check_*()` functions throughout our tidyverse packages, and one of those is `check_name()`, which conflicted with the previous dplyr function name. The `import-standalone-*.R` files that get brought in there are read only, so I'd prefer to not have to tweak them just to make arrow happy.
It seems like `dplyr:::check_n_name()` is fairly simple, is it possible to just inline it in arrow directly? That seems to be the only place arrow does `getFromNamespace()` for dplyr, so it would be great to remove that entirely anyways.
### Component(s)
R
--
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: issues-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] DavisVaughan commented on issue #34366: Don't `getFromNamespace()` the `dplyr:::check_name()` helper
Posted by "DavisVaughan (via GitHub)" <gi...@apache.org>.
DavisVaughan commented on issue #34366:
URL: https://github.com/apache/arrow/issues/34366#issuecomment-1446652168
Since you already use both rlang and glue it looks like you can just add these two helpers to arrow itself (i.e. don't try and grab anything from dplyr's internals)
```r
check_n_name <- function(name,
vars,
call = caller_env()) {
if (is.null(name)) {
name <- n_name(vars)
if (name != "n") {
inform(c(
glue("Storing counts in `{name}`, as `n` already present in input"),
i = "Use `name = \"new_name\"` to pick a new name."
))
}
} else {
if (!is_string(name)) {
abort("`name` must be a string or `NULL`.", call = call)
}
}
name
}
n_name <- function(x) {
name <- "n"
while (name %in% x) {
name <- paste0("n", name)
}
name
}
```
that's all I meant by inlining
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] thisisnic closed issue #34366: Don't `getFromNamespace()` the `dplyr:::check_name()` helper
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic closed issue #34366: Don't `getFromNamespace()` the `dplyr:::check_name()` helper
URL: https://github.com/apache/arrow/issues/34366
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] thisisnic commented on issue #34366: Don't `getFromNamespace()` the `dplyr:::check_name()` helper
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on issue #34366:
URL: https://github.com/apache/arrow/issues/34366#issuecomment-1448457322
Issue resolved by pull request 34369
https://github.com/apache/arrow/pull/34369
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] DavisVaughan commented on issue #34366: Don't `getFromNamespace()` the `dplyr:::check_name()` helper
Posted by "DavisVaughan (via GitHub)" <gi...@apache.org>.
DavisVaughan commented on issue #34366:
URL: https://github.com/apache/arrow/issues/34366#issuecomment-1460139550
Thanks so much!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] thisisnic commented on issue #34366: Don't `getFromNamespace()` the `dplyr:::check_name()` helper
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on issue #34366:
URL: https://github.com/apache/arrow/issues/34366#issuecomment-1446627817
Thanks for letting us know about this!
I think the problem with inlining is that this would cause a CRAN check failure, unless the policy has been updated since I last checked? Regardless, there'll be some reasonable workaround we can do, and stop using `getFromNamespace("check_name", "dplyr") `.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] thisisnic commented on issue #34366: Don't `getFromNamespace()` the `dplyr:::check_name()` helper
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on issue #34366:
URL: https://github.com/apache/arrow/issues/34366#issuecomment-1459878668
@DavisVaughan We have now released arrow 11.0.0.3 which contains a fix for this, and I've just received the "on its way to CRAN" email, so it should be there shortly.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org