You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/03/16 03:16:47 UTC
[GitHub] [arrow] MrMallIronmaker opened a new pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
MrMallIronmaker opened a new pull request #12642:
URL: https://github.com/apache/arrow/pull/12642
Created a simple version of `rename_with` that applies the function to the current names of the .data argument and passes the result to `rename`.
This was a quick writeup on my end and passed a few smoke tests. I believe all the functions I've used in the new code are from base R or current imports like rlang, and the magic in editing the arrow data query is handled by `rename`.
--
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 a change in pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
thisisnic commented on a change in pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#discussion_r832246876
##########
File path: r/R/dplyr-select.R
##########
@@ -32,6 +32,16 @@ rename.arrow_dplyr_query <- function(.data, ...) {
}
rename.Dataset <- rename.ArrowTabular <- rename.RecordBatchReader <- rename.arrow_dplyr_query
+rename_with.arrow_dplyr_query <- function(.data, .fn, .cols = everything(), ...) {
+ .fn <- rlang::as_function(.fn)
+ old_names <- names(select(.data, {{ .cols }}))
+ new_names <- do.call(.fn, list(old_names))
+ rename_args <- c(list(.data), as.list(old_names))
+ names(rename_args) <- c(".data", new_names)
+ do.call(rename, rename_args)
Review comment:
```suggestion
rename(.data, !!set_names(old_names, new_names))
```
Minor change here to make this more `rlang` style in keeping with the other code in the package + avoiding use of `do.call`.
##########
File path: r/R/dplyr-select.R
##########
@@ -32,6 +32,16 @@ rename.arrow_dplyr_query <- function(.data, ...) {
}
rename.Dataset <- rename.ArrowTabular <- rename.RecordBatchReader <- rename.arrow_dplyr_query
+rename_with.arrow_dplyr_query <- function(.data, .fn, .cols = everything(), ...) {
+ .fn <- rlang::as_function(.fn)
+ old_names <- names(select(.data, {{ .cols }}))
+ new_names <- do.call(.fn, list(old_names))
+ rename_args <- c(list(.data), as.list(old_names))
+ names(rename_args) <- c(".data", new_names)
+ do.call(rename, rename_args)
Review comment:
```suggestion
rename(.data, !!set_names(old_names, new_names))
```
Minor change here to make this more `rlang` style in keeping with the other code in the package + avoiding use of `do.call()`.
--
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] github-actions[bot] commented on pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#issuecomment-1068688212
--
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] ursabot edited a comment on pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#issuecomment-1081212335
Benchmark runs are scheduled for baseline = ad7380e443de1f2a7becf597e0ee5f3b9c8f7f35 and contender = b781710d950f61912c707d899f4f8750f4e149b0. b781710d950f61912c707d899f4f8750f4e149b0 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/29af9846fb5d41d58b72f6c1c5ea48df...b7a62c6a58934bb2a599644a0246159a/)
[Finished :arrow_down:0.21% :arrow_up:0.08%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/25e9c90ee8e14e1399fb22a64510a613...817d9371b59c4e4fbb70b65f89491807/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/d48673cab775408594214e56f601c193...aa51f3e8df2c411db75b7ec2f9453f13/)
[Finished :arrow_down:0.26% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/c417854a965f4c5c9151dfb2c1c0e36b...f50945d9111c4177976c3ed4fc6263c1/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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] ursabot commented on pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#issuecomment-1081212335
Benchmark runs are scheduled for baseline = ad7380e443de1f2a7becf597e0ee5f3b9c8f7f35 and contender = b781710d950f61912c707d899f4f8750f4e149b0. b781710d950f61912c707d899f4f8750f4e149b0 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/29af9846fb5d41d58b72f6c1c5ea48df...b7a62c6a58934bb2a599644a0246159a/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/25e9c90ee8e14e1399fb22a64510a613...817d9371b59c4e4fbb70b65f89491807/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/d48673cab775408594214e56f601c193...aa51f3e8df2c411db75b7ec2f9453f13/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/c417854a965f4c5c9151dfb2c1c0e36b...f50945d9111c4177976c3ed4fc6263c1/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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] MrMallIronmaker commented on pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
MrMallIronmaker commented on pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#issuecomment-1076570658
Changes made as requested - thanks!
--
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] ursabot edited a comment on pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#issuecomment-1081212335
Benchmark runs are scheduled for baseline = ad7380e443de1f2a7becf597e0ee5f3b9c8f7f35 and contender = b781710d950f61912c707d899f4f8750f4e149b0. b781710d950f61912c707d899f4f8750f4e149b0 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/29af9846fb5d41d58b72f6c1c5ea48df...b7a62c6a58934bb2a599644a0246159a/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/25e9c90ee8e14e1399fb22a64510a613...817d9371b59c4e4fbb70b65f89491807/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/d48673cab775408594214e56f601c193...aa51f3e8df2c411db75b7ec2f9453f13/)
[Finished :arrow_down:0.26% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/c417854a965f4c5c9151dfb2c1c0e36b...f50945d9111c4177976c3ed4fc6263c1/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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] MrMallIronmaker commented on pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
MrMallIronmaker commented on pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#issuecomment-1071099459
I've written a few test cases for `rename_with` and hit the "todo" for using tidyselect with select, as well. I think this is good enough to be reviewed! Thanks.
--
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 a change in pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
thisisnic commented on a change in pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#discussion_r832249136
##########
File path: r/R/dplyr-select.R
##########
@@ -32,6 +32,16 @@ rename.arrow_dplyr_query <- function(.data, ...) {
}
rename.Dataset <- rename.ArrowTabular <- rename.RecordBatchReader <- rename.arrow_dplyr_query
+rename_with.arrow_dplyr_query <- function(.data, .fn, .cols = everything(), ...) {
+ .fn <- rlang::as_function(.fn)
Review comment:
In `arrow/r/R/arrow-package-R` we have a load of `rlang` function listed with `@importFrom` roxygen tags. Could you add `as_function` there and then call it directly here?
##########
File path: r/R/dplyr-select.R
##########
@@ -32,6 +32,16 @@ rename.arrow_dplyr_query <- function(.data, ...) {
}
rename.Dataset <- rename.ArrowTabular <- rename.RecordBatchReader <- rename.arrow_dplyr_query
+rename_with.arrow_dplyr_query <- function(.data, .fn, .cols = everything(), ...) {
+ .fn <- rlang::as_function(.fn)
+ old_names <- names(select(.data, {{ .cols }}))
+ new_names <- do.call(.fn, list(old_names))
Review comment:
Let's remove the `do.call()` here. Perhaps `.fn(old_names)` could work?
##########
File path: r/R/dplyr-select.R
##########
@@ -32,6 +32,16 @@ rename.arrow_dplyr_query <- function(.data, ...) {
}
rename.Dataset <- rename.ArrowTabular <- rename.RecordBatchReader <- rename.arrow_dplyr_query
+rename_with.arrow_dplyr_query <- function(.data, .fn, .cols = everything(), ...) {
+ .fn <- rlang::as_function(.fn)
+ old_names <- names(select(.data, {{ .cols }}))
+ new_names <- do.call(.fn, list(old_names))
+ rename_args <- c(list(.data), as.list(old_names))
+ names(rename_args) <- c(".data", new_names)
+ do.call(rename, rename_args)
Review comment:
```suggestion
rename(.data, !!set_names(old_names, new_names))
```
Minor change here to make this more `rlang` style in keeping with the other code in the package.
--
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] MrMallIronmaker commented on pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
MrMallIronmaker commented on pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#issuecomment-1071099459
I've written a few test cases for `rename_with` and hit the "todo" for using tidyselect with select, as well. I think this is good enough to be reviewed! Thanks.
--
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] jonkeane closed pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
jonkeane closed pull request #12642:
URL: https://github.com/apache/arrow/pull/12642
--
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] ursabot edited a comment on pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#issuecomment-1081212335
Benchmark runs are scheduled for baseline = ad7380e443de1f2a7becf597e0ee5f3b9c8f7f35 and contender = b781710d950f61912c707d899f4f8750f4e149b0. b781710d950f61912c707d899f4f8750f4e149b0 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/29af9846fb5d41d58b72f6c1c5ea48df...b7a62c6a58934bb2a599644a0246159a/)
[Finished :arrow_down:0.21% :arrow_up:0.08%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/25e9c90ee8e14e1399fb22a64510a613...817d9371b59c4e4fbb70b65f89491807/)
[Failed :arrow_down:0.71% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/d48673cab775408594214e56f601c193...aa51f3e8df2c411db75b7ec2f9453f13/)
[Finished :arrow_down:0.26% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/c417854a965f4c5c9151dfb2c1c0e36b...f50945d9111c4177976c3ed4fc6263c1/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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 pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
thisisnic commented on pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#issuecomment-1068998700
Hi @MrMallIronmaker - thanks for submitting this PR! Unit tests are definitely the way to go in terms of next steps. Let us know if you have any questions about any of our set-up!
--
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] markromanmiller commented on pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
markromanmiller commented on pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#issuecomment-1079068936
I changed the spacing manually based on the lintr job output. `remotes::install_github` wasn't able to download the tarball of the lintr branch mentioned in the workflow setup, but that issue it seems to be a Github thing? I wasn't able to download it either through the web interface. In any case, I was able to run the standard lintr, and there were no linting messages in the files I've changed.
--
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] ursabot edited a comment on pull request #12642: ARROW-15947: [R] rename_with s3 method for arrow_dplyr_query
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12642:
URL: https://github.com/apache/arrow/pull/12642#issuecomment-1081212335
Benchmark runs are scheduled for baseline = ad7380e443de1f2a7becf597e0ee5f3b9c8f7f35 and contender = b781710d950f61912c707d899f4f8750f4e149b0. b781710d950f61912c707d899f4f8750f4e149b0 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/29af9846fb5d41d58b72f6c1c5ea48df...b7a62c6a58934bb2a599644a0246159a/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/25e9c90ee8e14e1399fb22a64510a613...817d9371b59c4e4fbb70b65f89491807/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/d48673cab775408594214e56f601c193...aa51f3e8df2c411db75b7ec2f9453f13/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/c417854a965f4c5c9151dfb2c1c0e36b...f50945d9111c4177976c3ed4fc6263c1/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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