You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "thisisnic (via GitHub)" <gi...@apache.org> on 2023/05/09 07:35:37 UTC
[GitHub] [arrow] thisisnic opened a new pull request, #35502: GH-32282: [R] Update case_when() binding to match changes in dplyr
thisisnic opened a new pull request, #35502:
URL: https://github.com/apache/arrow/pull/35502
This PR implements the `.default` argument for `case_when` and raises errors if non-NULL values are supplied for `.ptype` and `.size`.
--
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 #35502: GH-32282: [R] Update case_when() binding to match changes in dplyr
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35502:
URL: https://github.com/apache/arrow/pull/35502#issuecomment-1544827565
['Python', 'R'] benchmarks have high level of regressions.
[test-mac-arm](https://conbench.ursa.dev/compare/runs/1403d9492f8b440dbee1d83d19b829eb...9648ebda147f4392be54162295f0c572/)
--
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] eitsupi commented on a diff in pull request #35502: GH-32282: [R] Update case_when() binding to match changes in dplyr
Posted by "eitsupi (via GitHub)" <gi...@apache.org>.
eitsupi commented on code in PR #35502:
URL: https://github.com/apache/arrow/pull/35502#discussion_r1188446245
##########
r/R/dplyr-funcs-conditional.R:
##########
@@ -90,7 +90,15 @@ register_bindings_conditional <- function() {
out
})
- register_binding("dplyr::case_when", function(...) {
+ register_binding("dplyr::case_when", function(..., .default = NULL, .ptype = NULL, .size = NULL) {
Review Comment:
> raises errors if non-NULL values are supplied for .ptype and .size.
Would it be better to mention the limitation with the `notes`?
https://github.com/apache/arrow/blob/7f3e87d2d5e6f0568dbb92b468d911717d9aed91/r/R/dplyr-funcs.R#L63-L64
--
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] eitsupi commented on pull request #35502: GH-32282: [R] Update case_when() binding to match changes in dplyr
Posted by "eitsupi (via GitHub)" <gi...@apache.org>.
eitsupi commented on PR #35502:
URL: https://github.com/apache/arrow/pull/35502#issuecomment-1542378902
Doesn't the note need to be described this way?
https://github.com/apache/arrow/blob/df5f67d5a80c676417c38563743e72b17e854f1f/r/R/dplyr-funcs-datetime.R#L81-L84
--
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 merged pull request #35502: GH-32282: [R] Update case_when() binding to match changes in dplyr
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic merged PR #35502:
URL: https://github.com/apache/arrow/pull/35502
--
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 diff in pull request #35502: GH-32282: [R] Update case_when() binding to match changes in dplyr
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #35502:
URL: https://github.com/apache/arrow/pull/35502#discussion_r1188681486
##########
r/R/dplyr-funcs-conditional.R:
##########
@@ -90,7 +90,15 @@ register_bindings_conditional <- function() {
out
})
- register_binding("dplyr::case_when", function(...) {
+ register_binding("dplyr::case_when", function(..., .default = NULL, .ptype = NULL, .size = NULL) {
Review Comment:
Doing both makes sense
--
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 #35502: GH-32282: [R] Update case_when() binding to match changes in dplyr
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on PR #35502:
URL: https://github.com/apache/arrow/pull/35502#issuecomment-1539611800
We could implement `.ptype` in a follow-up ticket if there is user interest in it. A possible implementation might involve calling cast on the `case_when` expression to cast it to the correct type. We might want to use `infer_type()` so users can pass in e.g. `integer()` like in the `.ptype` example in the dplyr tests. To do this, we can implement `infer_type.DataType` which just passes the input object back out - then users can input either R objects or Arrow datatypes to this function.
--
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 #35502: GH-32282: [R] Update case_when() binding to match changes in dplyr
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35502:
URL: https://github.com/apache/arrow/pull/35502#issuecomment-1544826586
Benchmark runs are scheduled for baseline = dec445374cd0dd4a2fd9808bfeb7a7e0162c9e14 and contender = 1a038adad85be6f7e6949cc700dcb9b211feb44d. 1a038adad85be6f7e6949cc700dcb9b211feb44d 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/ea8858d4710e419287df1e80d50a213d...2a1019bef15540e195ac2b45f22f71c0/)
[Finished :arrow_down:2.76% :arrow_up:0.03%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/1403d9492f8b440dbee1d83d19b829eb...9648ebda147f4392be54162295f0c572/)
[Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/5e2440a4022f4b69a299a0b1d4551575...3ef9ee03999c4ea188de94b5488a5665/)
[Finished :arrow_down:0.42% :arrow_up:0.06%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/153232e68ba348f88a4bf6aedde388f6...d14230f22162403ab43f96c6fc95b7fc/)
Buildkite builds:
[Finished] [`1a038ada` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2852)
[Finished] [`1a038ada` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2888)
[Failed] [`1a038ada` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2850)
[Finished] [`1a038ada` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2878)
[Finished] [`dec44537` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2851)
[Finished] [`dec44537` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2887)
[Finished] [`dec44537` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2854)
[Finished] [`dec44537` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2877)
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] github-actions[bot] commented on pull request #35502: GH-32282: [R] Update case_when() binding to match changes in dplyr
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35502:
URL: https://github.com/apache/arrow/pull/35502#issuecomment-1539610764
:warning: GitHub issue #32282 **has been automatically assigned in GitHub** to PR creator.
--
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 #35502: GH-32282: [R] Update case_when() binding to match changes in dplyr
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35502:
URL: https://github.com/apache/arrow/pull/35502#issuecomment-1539610720
* Closes: #32282
--
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