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/04/06 13:19:22 UTC

[GitHub] [arrow] ianmcook opened a new pull request #9907: ARROW-12227: [R] Fix nightly build failures caused by gsub and quantile [WIP]

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


   


-- 
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 closed pull request #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures

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


   


-- 
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 removed a comment on pull request #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

Posted by GitBox <gi...@apache.org>.
ianmcook removed a comment on pull request #9907:
URL: https://github.com/apache/arrow/pull/9907#issuecomment-814114892


   @github-actions crossbow submit test-r-minimal-build


-- 
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 pull request #9907: ARROW-12227: [R] Fix nightly build failures caused by gsub and quantile [WIP]

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


   @github-actions crossbow submit test-r-minimal-build


-- 
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 #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

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



##########
File path: r/tests/testthat/test-dplyr-string-functions.R
##########
@@ -18,6 +18,27 @@
 library(dplyr)
 library(stringr)
 
+test_that("sub and gsub with ignore.case = FALSE and fixed = TRUE", {
+  df <- tibble(x = c("Foo", "bar"))

Review comment:
       Added in da7defb53. Thanks for catching that!




-- 
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 #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

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



##########
File path: r/tests/testthat/test-compute-aggregate.R
##########
@@ -267,6 +260,20 @@ test_that("quantile and median NAs, edge cases, and exceptions", {
   )
 })
 
+test_that("median passes ... args to quantile", {
+  skip_if(
+    !"..." %in% names(formals(median)),
+    "The median generic lacks dots in R 3.3.0 and earlier"
+  )

Review comment:
       I'm surprised that `R CMD check` did not detect S3 generic/method consistency problems in R 3.3.0, but it didn't:
   https://github.com/ursacomputing/crossbow/runs/2275559899#step:7:193
   Given that, it's cleaner to skip the tests that use the `median()` dots args than it would be to conditionally define `median.ArrowDatum` with/without dots depending on whether the generic has them.




-- 
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 #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

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



##########
File path: r/tests/testthat/test-compute-aggregate.R
##########
@@ -267,6 +260,20 @@ test_that("quantile and median NAs, edge cases, and exceptions", {
   )
 })
 
+test_that("median passes ... args to quantile", {
+  skip_if(
+    !"..." %in% names(formals(median)),
+    "The median generic lacks dots in R 3.3.0 and earlier"
+  )

Review comment:
       In ARROW-11338, I assumed the `median()` generic always had dots args, but apparently it didn't in R 3.3.0 and earlier.
   
   I'm surprised that `R CMD check` did not detect S3 generic/method consistency problems in R 3.3.0, but it did not:
   https://github.com/ursacomputing/crossbow/runs/2275559899#step:7:193
   Given that, it's cleaner to skip the tests that use the `median()` dots args than it would be to conditionally define `median.ArrowDatum` with/without dots depending on whether the generic has them.




-- 
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 removed a comment on pull request #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

Posted by GitBox <gi...@apache.org>.
ianmcook removed a comment on pull request #9907:
URL: https://github.com/apache/arrow/pull/9907#issuecomment-814160936


   @github-actions crossbow submit test-r-minimal-build test-r-versions


-- 
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 #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures

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


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


-- 
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 #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

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



##########
File path: r/tests/testthat/test-compute-aggregate.R
##########
@@ -267,6 +260,20 @@ test_that("quantile and median NAs, edge cases, and exceptions", {
   )
 })
 
+test_that("median passes ... args to quantile", {
+  skip_if(
+    !"..." %in% names(formals(median)),
+    "The median generic lacks dots in R 3.3.0 and earlier"
+  )

Review comment:
       I'm surprised that `R CMD check` did not detect S3 generic/method consistency problems in R 3.3.0, but it didn't:
   https://github.com/ursacomputing/crossbow/runs/2275559899#step:7:193
   Given that, it's simpler to skip the tests that use the `median()` dots args than it would be to conditionally define `median.ArrowDatum` with/without dots depending on whether the generic has them.




-- 
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 #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

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



##########
File path: r/tests/testthat/test-dplyr-string-functions.R
##########
@@ -18,6 +18,27 @@
 library(dplyr)
 library(stringr)
 
+test_that("sub and gsub with ignore.case = FALSE and fixed = TRUE", {
+  df <- tibble(x = c("Foo", "bar"))

Review comment:
       Yes, they do depend on utf8proc. I'll add that at the top.




-- 
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 pull request #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures

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


   @github-actions crossbow submit test-r-minimal-build test-r-versions


-- 
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 #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

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



##########
File path: r/tests/testthat/test-dplyr-string-functions.R
##########
@@ -18,6 +18,27 @@
 library(dplyr)
 library(stringr)
 
+test_that("sub and gsub with ignore.case = FALSE and fixed = TRUE", {
+  df <- tibble(x = c("Foo", "bar"))

Review comment:
       Do these tests require skip_if_not_available("utf8proc")?

##########
File path: r/tests/testthat/test-dplyr-string-functions.R
##########
@@ -18,6 +18,27 @@
 library(dplyr)
 library(stringr)
 
+test_that("sub and gsub with ignore.case = FALSE and fixed = TRUE", {
+  df <- tibble(x = c("Foo", "bar"))
+
+  expect_dplyr_equal(
+    input %>%
+      transmute(x = sub("Foo", "baz", x, fixed = TRUE)) %>%
+      collect(),
+    df
+  )
+  expect_dplyr_equal(
+    input %>%
+      transmute(x = gub("o", "u", x, fixed = TRUE)) %>%

Review comment:
       ```suggestion
         transmute(x = gsub("o", "u", x, fixed = TRUE)) %>%
   ```




-- 
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 #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

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



##########
File path: r/tests/testthat/test-dplyr-string-functions.R
##########
@@ -18,6 +18,27 @@
 library(dplyr)
 library(stringr)
 
+test_that("sub and gsub with ignore.case = FALSE and fixed = TRUE", {
+  df <- tibble(x = c("Foo", "bar"))
+
+  expect_dplyr_equal(
+    input %>%
+      transmute(x = sub("Foo", "baz", x, fixed = TRUE)) %>%
+      collect(),
+    df
+  )
+  expect_dplyr_equal(
+    input %>%
+      transmute(x = gub("o", "u", x, fixed = TRUE)) %>%

Review comment:
       lol 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.

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



[GitHub] [arrow] ianmcook commented on pull request #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

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


   @github-actions crossbow submit test-r-minimal-build test-r-versions


-- 
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 #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures

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


   Revision: b9add3604be49bfc6a47a11358eb1787bd47bc8f
   
   Submitted crossbow builds: [ursacomputing/crossbow @ actions-266](https://github.com/ursacomputing/crossbow/branches/all?query=actions-266)
   
   |Task|Status|
   |----|------|
   |test-r-minimal-build|[![Azure](https://dev.azure.com/ursacomputing/crossbow/_apis/build/status/ursacomputing.crossbow?branchName=actions-266-azure-test-r-minimal-build)](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-266-azure-test-r-minimal-build)|
   |test-r-versions|[![Github Actions](https://github.com/ursacomputing/crossbow/workflows/Crossbow/badge.svg?branch=actions-266-github-test-r-versions)](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-266-github-test-r-versions)|


-- 
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 #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

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



##########
File path: r/tests/testthat/test-dplyr-string-functions.R
##########
@@ -18,6 +18,27 @@
 library(dplyr)
 library(stringr)
 
+test_that("sub and gsub with ignore.case = FALSE and fixed = TRUE", {
+  df <- tibble(x = c("Foo", "bar"))
+
+  expect_dplyr_equal(
+    input %>%
+      transmute(x = sub("Foo", "baz", x, fixed = TRUE)) %>%
+      collect(),
+    df
+  )
+  expect_dplyr_equal(
+    input %>%
+      transmute(x = gub("o", "u", x, fixed = TRUE)) %>%
+      collect(),
+    df
+  )
+
+})
+
+# many of the remainder of these tests require RE2
+skip_if_not_available("re2")
+

Review comment:
       In ARROW-11338, I forgot to add a check for RE2 before running tests that require it. Here I add that check, and also add a couple of simple tests before it that don't use RE2.




-- 
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 #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures [WIP]

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



##########
File path: r/tests/testthat/test-dplyr-string-functions.R
##########
@@ -18,6 +18,27 @@
 library(dplyr)
 library(stringr)
 
+test_that("sub and gsub with ignore.case = FALSE and fixed = TRUE", {
+  df <- tibble(x = c("Foo", "bar"))
+
+  expect_dplyr_equal(
+    input %>%
+      transmute(x = sub("Foo", "baz", x, fixed = TRUE)) %>%
+      collect(),
+    df
+  )
+  expect_dplyr_equal(
+    input %>%
+      transmute(x = gub("o", "u", x, fixed = TRUE)) %>%
+      collect(),
+    df
+  )
+
+})
+
+# many of the remainder of these tests require RE2
+skip_if_not_available("re2")
+

Review comment:
       In ARROW-11513, I forgot to add a check for RE2 before running tests that require it. Here I add that check, and also add a couple of simple tests before it that don't use RE2.




-- 
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 pull request #9907: ARROW-12227: [R] Fix RE2 and median nightly build failures

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


   The `test-r-versions` failure is spurious; the R 3.3 job is passing now (see https://github.com/ursacomputing/crossbow/runs/2280648483?check_suite_focus=true#step:7:234), but there was an unrelated timeout in the R 3.4 job.


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