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 2021/06/20 16:28:22 UTC

[GitHub] [arrow] ianmcook opened a new pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

ianmcook opened a new pull request #10563:
URL: https://github.com/apache/arrow/pull/10563


   This also resolves ARROW-13119: [R] Set empty schema in scalar Expressions


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #10563:
URL: https://github.com/apache/arrow/pull/10563#issuecomment-865064220


   Benchmark runs are scheduled for baseline = 9d1591781faa52598d30a4e6a3944bc893246d17 and contender = ad7891072cb63378b18bc95536543ffe7df96230. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Skipped :warning: Provided benchmark filters do not have any benchmark groups to be executed on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2 (mimalloc)](https://conbench.ursa.dev/compare/runs/4a8edd7990d84d4494679582e13fc191...6fce957e01cc484e991bb7976d22bf49/)
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x (mimalloc)](https://conbench.ursa.dev/compare/runs/6780d735c79949f8941cb26629837679...fed4cd9b81d144a78f6eb73f830ccf3a/)
   [Skipped :warning: Only ['C++'] langs are supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q (mimalloc)](https://conbench.ursa.dev/compare/runs/0475d49077764479a53a5adf5ca813a3...69981a59da6c4142aa758b4598513a0c/)
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ianmcook commented on a change in pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #10563:
URL: https://github.com/apache/arrow/pull/10563#discussion_r655739346



##########
File path: r/tests/testthat/test-dplyr.R
##########
@@ -800,6 +808,21 @@ test_that("type checks with is_*()", {
   )
 })
 
+test_that("type checks on expressions", {
+  expect_dplyr_equal(
+    input %>%
+      transmute(
+        a = is.character(as.character(int)),

Review comment:
       Yes—this is a simplistic and somewhat contrived example of the motivating use case. For a more compelling version of it, see ARROW-12055: To make our translation of `is.na()` work consistently with base R, we will need to run different code depending on whether its input has a real number type, and we can't do that if we don't know the type.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10563:
URL: https://github.com/apache/arrow/pull/10563#discussion_r655729044



##########
File path: r/tests/testthat/test-dplyr.R
##########
@@ -800,6 +808,21 @@ test_that("type checks with is_*()", {
   )
 })
 
+test_that("type checks on expressions", {
+  expect_dplyr_equal(
+    input %>%
+      transmute(
+        a = is.character(as.character(int)),

Review comment:
       I'm guessing this is the use case that's motivating all of these Expression/Schema issues? I wonder if there's another way to solve it in a more localized way, inside of the is* nse_funcs.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ianmcook closed pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

Posted by GitBox <gi...@apache.org>.
ianmcook closed pull request #10563:
URL: https://github.com/apache/arrow/pull/10563


   


-- 
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] ianmcook commented on pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #10563:
URL: https://github.com/apache/arrow/pull/10563#issuecomment-865063604


   @ursabot please benchmark lang=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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #10563:
URL: https://github.com/apache/arrow/pull/10563#issuecomment-865064220


   Benchmark runs are scheduled for baseline = 9d1591781faa52598d30a4e6a3944bc893246d17 and contender = ad7891072cb63378b18bc95536543ffe7df96230. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Skipped :warning: Provided benchmark filters do not have any benchmark groups to be executed on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2 (mimalloc)](https://conbench.ursa.dev/compare/runs/4a8edd7990d84d4494679582e13fc191...6fce957e01cc484e991bb7976d22bf49/)
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x (mimalloc)](https://conbench.ursa.dev/compare/runs/6780d735c79949f8941cb26629837679...fed4cd9b81d144a78f6eb73f830ccf3a/)
   [Skipped :warning: Only ['C++'] langs are supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q (mimalloc)](https://conbench.ursa.dev/compare/runs/0475d49077764479a53a5adf5ca813a3...69981a59da6c4142aa758b4598513a0c/)
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10563:
URL: https://github.com/apache/arrow/pull/10563#issuecomment-864579326


   https://issues.apache.org/jira/browse/ARROW-13117


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ianmcook commented on a change in pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #10563:
URL: https://github.com/apache/arrow/pull/10563#discussion_r655739346



##########
File path: r/tests/testthat/test-dplyr.R
##########
@@ -800,6 +808,21 @@ test_that("type checks with is_*()", {
   )
 })
 
+test_that("type checks on expressions", {
+  expect_dplyr_equal(
+    input %>%
+      transmute(
+        a = is.character(as.character(int)),

Review comment:
       Yes—this is a simplistic and somewhat contrived example of the motivating use case. For a more compelling version of it, see ARROW-12055: To make our translation of `is.na()` work consistently with base R, we will need to run different code depending on whether its input has a real number type, and we can't do that if we don't know the type.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] nealrichardson commented on a change in pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10563:
URL: https://github.com/apache/arrow/pull/10563#discussion_r655729044



##########
File path: r/tests/testthat/test-dplyr.R
##########
@@ -800,6 +808,21 @@ test_that("type checks with is_*()", {
   )
 })
 
+test_that("type checks on expressions", {
+  expect_dplyr_equal(
+    input %>%
+      transmute(
+        a = is.character(as.character(int)),

Review comment:
       I'm guessing this is the use case that's motivating all of these Expression/Schema issues? I wonder if there's another way to solve it in a more localized way, inside of the is* nse_funcs.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot commented on pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #10563:
URL: https://github.com/apache/arrow/pull/10563#issuecomment-865064220


   Benchmark runs are scheduled for baseline = 9d1591781faa52598d30a4e6a3944bc893246d17 and contender = ad7891072cb63378b18bc95536543ffe7df96230. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Skipped :warning: Provided benchmark filters do not have any benchmark groups to be executed on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2 (mimalloc)](https://conbench.ursa.dev/compare/runs/4a8edd7990d84d4494679582e13fc191...6fce957e01cc484e991bb7976d22bf49/)
   [Scheduled] [ursa-i9-9960x (mimalloc)](https://conbench.ursa.dev/compare/runs/6780d735c79949f8941cb26629837679...fed4cd9b81d144a78f6eb73f830ccf3a/)
   [Skipped :warning: Only ['C++'] langs are supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q (mimalloc)](https://conbench.ursa.dev/compare/runs/0475d49077764479a53a5adf5ca813a3...69981a59da6c4142aa758b4598513a0c/)
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ianmcook commented on a change in pull request #10563: ARROW-13117: [R] Retain schema in new Expressions

Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #10563:
URL: https://github.com/apache/arrow/pull/10563#discussion_r658842787



##########
File path: r/tests/testthat/test-dplyr.R
##########
@@ -800,6 +808,21 @@ test_that("type checks with is_*()", {
   )
 })
 
+test_that("type checks on expressions", {
+  expect_dplyr_equal(
+    input %>%
+      transmute(
+        a = is.character(as.character(int)),

Review comment:
       I created ARROW-13186 for future consideration of ways to implement type determination more cleanly




-- 
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