Skip to content

ARROW-16268: [R] Remove long-deprecated functions - #13550

Merged
nealrichardson merged 2 commits into
apache:masterfrom
nealrichardson:remove-deprecated
Jul 8, 2022
Merged

ARROW-16268: [R] Remove long-deprecated functions#13550
nealrichardson merged 2 commits into
apache:masterfrom
nealrichardson:remove-deprecated

Conversation

@nealrichardson

Copy link
Copy Markdown
Member

Also has a fix for the check NOTE about union_all and distinct.

@github-actions

Copy link
Copy Markdown

@paleolimbotpaleolimbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

There seems to be a CI failure (crash on 32-bit windows only on RTools 40?) but I don't see anything in this PR that could have introduced it.

This also might be a good opportunity to run devtools::document() with the latest roxygen2 since you're touching DESCRIPTION + a bunch of .Rd files.

@nealrichardson

Copy link
Copy Markdown
MemberAuthor

Windows CI crash looks like it may be related to #13521 cc @wesm

@wesm

wesm commented Jul 8, 2022

Copy link
Copy Markdown
Member

Sorry about that -- I see the error in #13521 but I misread the error that it looked like a flake

https://github.com/apache/arrow/runs/7243906864?check_suite_focus=true

What's the best way to diagnose so I can try to fix?

2022-07-08T15:24:46.8810386Z -- R CMD check results ----------------------------------- arrow 8.0.0.9000 ----
2022-07-08T15:24:46.8831977Z Duration: 13m 46.7s
2022-07-08T15:24:46.8832256Z 2022-07-08T15:24:46.8844886Z > running examples for arch 'i386' ... ERROR
2022-07-08T15:24:46.8845746Z ##[error] Running examples in 'arrow-Ex.R' failed
2022-07-08T15:24:46.8846746Z The error most likely occurred in:
2022-07-08T15:24:46.8847125Z 2022-07-08T15:24:46.8847625Z > base::assign(".ptime", proc.time(), pos = "CheckExEnv")
2022-07-08T15:24:46.8848050Z > ### Name: to_arrow
2022-07-08T15:24:46.8848826Z > ### Title: Create an Arrow object from others
2022-07-08T15:24:46.8849266Z > ### Aliases: to_arrow
2022-07-08T15:24:46.8849593Z > 2022-07-08T15:24:46.8849939Z > ### ** Examples
2022-07-08T15:24:46.8850499Z > 2022-07-08T15:24:46.8850822Z > ## Don't show: 2022-07-08T15:24:46.8851492Z > if (getFromNamespace("run_duckdb_examples", "arrow")()) (if (getRversion() >= "3.4") withAutoprint else force)({ # examplesIf
2022-07-08T15:24:46.8852006Z + ## End(Don't show)
2022-07-08T15:24:46.8852378Z + library(dplyr)
2022-07-08T15:24:46.8852697Z + 2022-07-08T15:24:46.8853111Z + ds <- InMemoryDataset$create(mtcars)
2022-07-08T15:24:46.8853471Z + 2022-07-08T15:24:46.8853775Z + ds %>%
2022-07-08T15:24:46.8854245Z + filter(mpg < 30) %>%
2022-07-08T15:24:46.8855417Z + to_duckdb() %>%
2022-07-08T15:24:46.8855793Z + group_by(cyl) %>%
2022-07-08T15:24:46.8856257Z + summarize(mean_mpg = mean(mpg, na.rm = TRUE)) %>%
2022-07-08T15:24:46.8856665Z + to_arrow() %>%
2022-07-08T15:24:46.8857014Z + collect()
2022-07-08T15:24:46.8857490Z + ## Don't show: 2022-07-08T15:24:46.8857874Z + }) # examplesIf
2022-07-08T15:24:46.8858223Z > library(dplyr)
2022-07-08T15:24:46.8858540Z 2022-07-08T15:24:46.8858911Z Attaching package: 'dplyr'
2022-07-08T15:24:46.8859246Z 2022-07-08T15:24:46.8859673Z The following objects are masked from 'package:stats':
2022-07-08T15:24:46.8860063Z 2022-07-08T15:24:46.8860414Z filter, lag
2022-07-08T15:24:46.8860726Z 2022-07-08T15:24:46.8861172Z The following objects are masked from 'package:base':
2022-07-08T15:24:46.8861529Z 2022-07-08T15:24:46.8861922Z intersect, setdiff, setequal, union
2022-07-08T15:24:46.8862255Z 2022-07-08T15:24:46.8862624Z > ds <- InMemoryDataset$create(mtcars)
2022-07-08T15:24:46.8863210Z > ds %>% filter(mpg < 30) %>% to_duckdb() %>% group_by(cyl) %>% summarize(mean_mpg = mean(mpg, 2022-07-08T15:24:46.8863726Z + na.rm = TRUE)) %>% to_arrow() %>% collect()

@nealrichardson

Copy link
Copy Markdown
MemberAuthor

I looked to the end of the verbose test output, where you see test-chunked-array running and then it dies after the assertion on L260: https://github.com/apache/arrow/runs/7243906864?check_suite_focus=true#step:11:6360

So I checked out what's happening in the test. By my reading of the output, it does not always occur, but when it does, it happens when you Filter an empty ChunkedArray with another (boolean) empty ChunkedArray: https://github.com/apache/arrow/blob/master/r/tests/testthat/test-chunked-array.R#L265

@wesm

wesm commented Jul 8, 2022

Copy link
Copy Markdown
Member

Ok I think that gives me enough to go on I’ll try to fix.

@paleolimbot

Copy link
Copy Markdown
Member

I have another PR crashing at this line on Windows:

expect_as_vector(a[rep(c(TRUE, FALSE), 5)], vec[c(1, 3, 5, 7, 9)])

(seems also to be subsetting using a boolean)

@nealrichardson
nealrichardson merged commit a48c09e into apache:masterJul 8, 2022
@nealrichardson
nealrichardson deleted the remove-deprecated branch July 8, 2022 17:27
@wesm

wesm commented Jul 8, 2022

Copy link
Copy Markdown
Member

I've spent about an hour tinkering on this and I feel somewhat powerless to debug the problem. Are there instructions about how to debug the mingw32 RTools 4.0 C++ build on Windows? We don't run the C++ unit tests when building the Arrow libraries, and my guess is that running arrow-compute-vector-test in a debug build will reveal the issue (and with mingw32 we can use gdb to find out where the issue is coming from)? If there's someone better equipped to help identify the issue I would really appreciate it.

@paleolimbot

Copy link
Copy Markdown
Member

I hate to volunteer @wjones127 for what is probably a hard and possibly time-consuming debugging problem, but I personally consider him the master of debugging R packages on Windows.

@wesm

wesm commented Jul 8, 2022

Copy link
Copy Markdown
Member

I don't have access to a Windows VM right now otherwise I would try to do it myself, but it should be sufficient to build the C++ library locally with the mingw32 RTools toolchain and run the unit test suite -- building the R package may not be needed

@wjones127

Copy link
Copy Markdown
Member

I haven't yet built the 32-bit version, but I will look into that now. My instructions for 64-bit are here FWIW: https://www.datawill.io/2022/04/02/windows-apache-arrow-development-environment-with-rtools-4-0/

@wesm

wesm commented Jul 10, 2022

Copy link
Copy Markdown
Member

Just confirming here also that 88b42ef fixed the issue

kou pushed a commit that referenced this pull request Feb 20, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@nealrichardson@wesm@paleolimbot@wjones127