romainfrancois commented on a change in pull request #11404:
URL: https://github.com/apache/arrow/pull/11404#discussion_r728885094
##########
File path: r/src/array_to_vector.cpp
##########
@@ -65,11 +65,14 @@ class Converter {
// converter is passed as self to outlive the scope of Converter::Convert()
SEXP ScheduleConvertTasks(RTasks& tasks, std::shared_ptr<Converter> self) {
- // try altrep first
+// try altrep first
+#if defined(HAS_ALTREP)
+ void Init_Altrep_classes(DllInfo * dll);
SEXP alt = altrep::MakeAltrepVector(chunked_array_);
Review comment:
`MakeAltrepVector()` is defined no matter what so that only conditional
compiling would only happen in `altrep.cpp` and so it always returns
`R_NilValue` when altrep is not defined, which signals that an altrep vector
could not be made and so to do the normal thing
##########
File path: r/tests/testthat/test-Array.R
##########
@@ -729,6 +729,9 @@ test_that("Handling string data with embedded nuls", {
)
array_with_nul <- Array$create(raws)$cast(utf8())
+ # test below use/rely on altrep
+ skip_if_r_version("3.5.0")
Review comment:
🤔 we can skip them for now, but we still would expect errors (perhaps
not in the same order) if altrep was not available
##########
File path: r/tests/testthat/test-chunked-array.R
##########
@@ -431,6 +431,12 @@ test_that("Handling string data with embedded nuls", {
class = c("arrow_binary", "vctrs_vctr", "list")
)
chunked_array_with_nul <- ChunkedArray$create(raws)$cast(utf8())
+
+ # The behavior of the warnings/errors is slightly different with and without
+ # altrep. Without it (i.e. 3.5.0 and below, the error would trigger
immediately
+ # on `as.vector()` where as with it, the error only happens on
materialization)
+ skip_if_r_version("3.5.0")
Review comment:
Same. I guess skipping is fine, but otherwise we can rework the tests to
better stress versions without altrep.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
