You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "paleolimbot (via GitHub)" <gi...@apache.org> on 2025/11/19 20:50:42 UTC
[PR] fix(r): Add explicit Set ELT ALTREP method [arrow-nanoarrow]
paleolimbot opened a new pull request, #826:
URL: https://github.com/apache/arrow-nanoarrow/pull/826
From CRAN:
```
══ Failed tests ════════════════════════════════════════════════════════════════
── Failure ('test-altrep.R:39:3'): nanoarrow_altrep_chr() works for string ─────
Expected `is_nanoarrow_altrep_materialized(x_altrep)` to be TRUE.
Differences:
`actual`: FALSE
`expected`: TRUE
── Failure ('test-altrep.R:46:3'): nanoarrow_altrep_chr() works for string ─────
Expected output from `.Internal(inspect(x_altrep))` to match regexp "<materialized nanoarrow::altrep_chr\\[27\\]>".
Actual output:
x | @559139743bb8 16 STRSXP g0c0 [MARK,REF(65535)] <nanoarrow::altrep_chr[27]>
[ FAIL 2 | WARN 0 | SKIP 8 | PASS 1610 ]
Error:
! Test failures.
Execution halted
```
This is happening for all r-devel.
We'd been relying on an omitted SetELT method to automatically materialize because of a DATAPTR call; however, this appears to have been changed.
--
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
Re: [PR] fix(r): Fix test for forthcoming ALTREP behaviour in R-devel [arrow-nanoarrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #826:
URL: https://github.com/apache/arrow-nanoarrow/pull/826#discussion_r2546872005
##########
r/tests/testthat/test-altrep.R:
##########
@@ -32,21 +32,29 @@ test_that("nanoarrow_altrep_chr() works for string", {
expect_length(x_altrep, 27)
expect_false(is_nanoarrow_altrep_materialized(x_altrep))
- # Setting an element will materialize, duplicate, then modify
+ # Setting an element will materialize, duplicate, then modify in R <= 4.5
x_altrep2 <- x_altrep
x_altrep2[1] <- "not a letter"
expect_identical(x_altrep2, c("not a letter", letters))
- expect_true(is_nanoarrow_altrep_materialized(x_altrep))
- # Check the same operations on the materialized output
+ # Materialization should get printed in inspect() if it occurred here (R <= 4.5)
+ # On R-devel (2025-11-19), x_altrep2 becomes an independent copy here and
+ # the original is not materialized with our current implementation.
+ if (getRversion() <= "4.5.1") {
+ expect_true(is_nanoarrow_altrep_materialized(x_altrep))
+ expect_output(.Internal(inspect(x_altrep)), "<materialized nanoarrow::altrep_chr\\[27\\]>")
+ }
Review Comment:
This is the crux of the fix
--
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
Re: [PR] fix(r): Fix test for forthcoming ALTREP behaviour in R-devel [arrow-nanoarrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot merged PR #826:
URL: https://github.com/apache/arrow-nanoarrow/pull/826
--
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