Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI][R][Python] R and Python integration's tests failed #39933

Closed
kou opened this issue Feb 4, 2024 · 15 comments · Fixed by #39969
Closed

[CI][R][Python] R and Python integration's tests failed #39933

kou opened this issue Feb 4, 2024 · 15 comments · Fixed by #39969

Comments

@kou
Copy link
Member

kou commented Feb 4, 2024

Describe the bug, including details regarding any error messages, version, and platform.

https://github.com/apache/arrow/actions/runs/7760862085/job/21168058007#step:6:32228

══ Failed tests ════════════════════════════════════════════════════════════════
── Error ('test-python-flight.R:32:5'): flight_put ─────────────────────────────
<python.builtin.ValueError/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: ValueError: PyCapsule_GetPointer called with incorrect name
Run `reticulate::py_last_error()` for details.
Backtrace:
    ▆
 1. └─arrow::flight_put(client, example_data, path = flight_obj) at test-python-flight.R:32:5
 2.   ├─reticulate::r_to_py(data)
 3.   └─arrow:::r_to_py.Table(data)
 4.     ├─reticulate::r_to_py(as_record_batch_reader(x), convert = FALSE)
 5.     └─arrow:::r_to_py.RecordBatchReader(...)
 6.       └─pa$lib$RecordBatchReader$`_import_from_c`(pyarrow_compatible_pointer(stream_ptr))
 7.         └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)
── Error ('test-python-flight.R:42:5'): flight_put with max_chunksize ──────────
<python.builtin.ValueError/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: ValueError: PyCapsule_GetPointer called with incorrect name
Run `reticulate::py_last_error()` for details.
Backtrace:
    ▆
 1. └─arrow::flight_put(client, example_data, path = flight_obj, max_chunksize = 1) at test-python-flight.R:42:5
 2.   ├─reticulate::r_to_py(data)
 3.   └─arrow:::r_to_py.Table(data)
 3.   └─pa$Field$`_import_from_c`(pyarrow_compatible_pointer(schema_ptr))
 4.     └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)
── Error ('test-python.R:157:3'): RecordBatchReader to python ──────────────────
<python.builtin.ValueError/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: ValueError: PyCapsule_GetPointer called with incorrect name
Run `reticulate::py_last_error()` for details.
Backtrace:
    ▆
 1. ├─reticulate::r_to_py(reader) at test-python.R:157:3
 2. └─arrow:::r_to_py.RecordBatchReader(reader)
 3.   └─pa$lib$RecordBatchReader$`_import_from_c`(pyarrow_compatible_pointer(stream_ptr))
 4.     └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)
── Error ('test-python.R:177:3'): RecordBatchReader from python ────────────────
<python.builtin.ValueError/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: ValueError: PyCapsule_GetPointer called with incorrect name
Run `reticulate::py_last_error()` for details.
Backtrace:
    ▆
 1. ├─reticulate::r_to_py(reader) at test-python.R:177:3
 2. └─arrow:::r_to_py.RecordBatchReader(reader)
 3.   └─pa$lib$RecordBatchReader$`_import_from_c`(pyarrow_compatible_pointer(stream_ptr))
 4.     └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)

The last success commit: 2721134
The first failed commit: b684028

Commits between them:

@jorisvandenbossche Could you take a look at this PyCapsule related error messages?

Component(s)

Continuous Integration, Python, R

@kou
Copy link
Member Author

kou commented Feb 5, 2024

#39942 may be happen by the same cause.

@amoeba
Copy link
Member

amoeba commented Feb 6, 2024

The timing of this failure starting to show up is around the time version 1.35.0 of reticulate came out and it looks like 1.35.0 was used in during the above failures.

@kou
Copy link
Member Author

kou commented Feb 6, 2024

Can we pin reticulate for now?
Could you report this to reticulate?

@amoeba
Copy link
Member

amoeba commented Feb 6, 2024

Let me verify my guess, I'll report back.

@amoeba
Copy link
Member

amoeba commented Feb 6, 2024

I ran the tests in ./r/tests/testthat/test-python-flight.R with reticulate 1.34.0 and 1.35.0 and get different failures so I think this needs a deeper investigation.

reticulate 1.34.0
==> Testing R file using 'testthat'Loading arrow
[ FAIL 6 | WARN 0 | SKIP 1 | PASS 1 ]

── Error (test-python-flight.R:28:5): flight_path_exists ───────────────────────
<pyarrow.lib.ArrowInvalid/python.builtin.ValueError/pyarrow.lib.ArrowException/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_iterate(it, f)`: pyarrow.lib.ArrowInvalid: invalid literal for int() with base 10: ''. Detail: Python exception: ValueError
Run `reticulate::py_last_error()` for details.
Backtrace:1. ├─testthat::expect_false(flight_obj %in% list_flights(client)) at test-python-flight.R:28:5
 2. │ └─testthat::quasi_label(enquo(object), label, arg = "object")
 3. │   └─rlang::eval_bare(expr, quo_get_env(quo))
 4. ├─flight_obj %in% list_flights(client)
 5. └─arrow::list_flights(client)
 6.   └─reticulate::iterate(generator, function(x) as.character(x$descriptor$path[[1]])) at r/R/flight.R:121:3
 7.     └─reticulate:::py_iterate(it, f)

── Error (test-python-flight.R:33:5): flight_put ───────────────────────────────
<pyarrow.lib.ArrowInvalid/python.builtin.ValueError/pyarrow.lib.ArrowException/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: pyarrow.lib.ArrowInvalid: invalid literal for int() with base 10: ''. Detail: Python exception: ValueError
Run `reticulate::py_last_error()` for details.
Backtrace:1. ├─testthat::expect_true(flight_path_exists(client, flight_obj)) at test-python-flight.R:33:5
  2. │ └─testthat::quasi_label(enquo(object), label, arg = "object")
  3. │   └─rlang::eval_bare(expr, quo_get_env(quo))
  4. └─arrow::flight_path_exists(client, flight_obj)
  5.   ├─base::tryCatch(...) at r/R/flight.R:128:3
  6.   │ └─base (local) tryCatchList(expr, classes, parentenv, handlers)
  7.   │   └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
  8.   │     └─base (local) doTryCatch(return(expr), name, parentenv, handler)
  9.   └─client$get_flight_info(descriptor_for_path(path)) at r/R/flight.R:130:7
 10.     └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)

── Error (test-python-flight.R:43:5): flight_put with max_chunksize ────────────
<pyarrow.lib.ArrowInvalid/python.builtin.ValueError/pyarrow.lib.ArrowException/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: pyarrow.lib.ArrowInvalid: invalid literal for int() with base 10: ''. Detail: Python exception: ValueError
Run `reticulate::py_last_error()` for details.
Backtrace:1. ├─testthat::expect_true(flight_path_exists(client, flight_obj)) at test-python-flight.R:43:5
  2. │ └─testthat::quasi_label(enquo(object), label, arg = "object")
  3. │   └─rlang::eval_bare(expr, quo_get_env(quo))
  4. └─arrow::flight_path_exists(client, flight_obj)
  5.   ├─base::tryCatch(...) at r/R/flight.R:128:3
  6.   │ └─base (local) tryCatchList(expr, classes, parentenv, handlers)
  7.   │   └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
  8.   │     └─base (local) doTryCatch(return(expr), name, parentenv, handler)
  9.   └─client$get_flight_info(descriptor_for_path(path)) at r/R/flight.R:130:7
 10.     └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)

── Error (test-python-flight.R:56:5): flight_get ───────────────────────────────
<pyarrow.lib.ArrowInvalid/python.builtin.ValueError/pyarrow.lib.ArrowException/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: pyarrow.lib.ArrowInvalid: invalid literal for int() with base 10: ''. Detail: Python exception: ValueError
Run `reticulate::py_last_error()` for details.
Backtrace:1. ├─expect_equal_data_frame(flight_get(client, flight_obj), example_data) at test-python-flight.R:56:5
 2. │ ├─expect_equal(as.data.frame(x), as.data.frame(y), ...) at tests/testthat/helper-expectation.R:24:3
 3. │ │ └─base::inherits(object, "ArrowObject") at tests/testthat/helper-expectation.R:35:3
 4. │ └─base::as.data.frame(x)
 5. └─arrow::flight_get(client, flight_obj)
 6.   └─arrow:::flight_reader(client, path) at r/R/flight.R:96:3
 7.     └─client$get_flight_info(descriptor_for_path(path)) at r/R/flight.R:102:3
 8.       └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)

── Error (test-python-flight.R:62:5): flight_put with RecordBatch ──────────────
<pyarrow.lib.ArrowInvalid/python.builtin.ValueError/pyarrow.lib.ArrowException/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: pyarrow.lib.ArrowInvalid: invalid literal for int() with base 10: ''. Detail: Python exception: ValueError
Run `reticulate::py_last_error()` for details.
Backtrace:1. ├─expect_equal_data_frame(flight_get(client, flight_obj2), example_data) at test-python-flight.R:62:5
 2. │ ├─expect_equal(as.data.frame(x), as.data.frame(y), ...) at tests/testthat/helper-expectation.R:24:3
 3. │ │ └─base::inherits(object, "ArrowObject") at tests/testthat/helper-expectation.R:35:3
 4. │ └─base::as.data.frame(x)
 5. └─arrow::flight_get(client, flight_obj2)
 6.   └─arrow:::flight_reader(client, path) at r/R/flight.R:96:3
 7.     └─client$get_flight_info(descriptor_for_path(path)) at r/R/flight.R:102:3
 8.       └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)

── Error (test-python-flight.R:66:5): flight_put with overwrite = FALSE ────────
<pyarrow.lib.ArrowInvalid/python.builtin.ValueError/pyarrow.lib.ArrowException/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: pyarrow.lib.ArrowInvalid: invalid literal for int() with base 10: ''. Detail: Python exception: ValueError
Run `reticulate::py_last_error()` for details.
Backtrace:1. ├─testthat::expect_error(...) at test-python-flight.R:66:5
  2. │ └─testthat:::expect_condition_matching(...)
  3. │   └─testthat:::quasi_capture(...)
  4. │     ├─testthat (local) .capture(...)
  5. │     │ └─base::withCallingHandlers(...)
  6. │     └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
  7. └─arrow::flight_put(...)
  8.   └─arrow::flight_path_exists(client, path) at r/R/flight.R:66:3
  9.     ├─base::tryCatch(...) at r/R/flight.R:128:3
 10.     │ └─base (local) tryCatchList(expr, classes, parentenv, handlers)
 11.     │   └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
 12.     │     └─base (local) doTryCatch(return(expr), name, parentenv, handler)
 13.     └─client$get_flight_info(descriptor_for_path(path)) at r/R/flight.R:130:7
 14.       └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)

── Skipped tests (1) ───────────────────────────────────────────────────────────────────────────────────────────────
• empty test (1): test-python-flight.R:75:3

[ FAIL 6 | WARN 0 | SKIP 1 | PASS 1 ]

Test complete
reticulate 1.35.0
==> Testing R file using 'testthat'Loading arrow
[ FAIL 5 | WARN 0 | SKIP 1 | PASS 2 ]

── Error (test-python-flight.R:32:5): flight_put ───────────────────────────────
<python.builtin.ValueError/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: ValueError: PyCapsule_GetPointer called with incorrect name
Run `reticulate::py_last_error()` for details.
Backtrace:1. └─arrow::flight_put(client, example_data, path = flight_obj) at test-python-flight.R:32:5
 2.   ├─reticulate::r_to_py(data) at r/R/flight.R:73:3
 3.   └─arrow:::r_to_py.Table(data)
 4.     ├─reticulate::r_to_py(as_record_batch_reader(x), convert = FALSE) at r/R/python.R:111:3
 5.     └─arrow:::r_to_py.RecordBatchReader(...)
 6.       └─pa$lib$RecordBatchReader$`_import_from_c`(pyarrow_compatible_pointer(stream_ptr)) at r/R/python.R:227:3
 7.         └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)

── Error (test-python-flight.R:42:5): flight_put with max_chunksize ────────────
<python.builtin.ValueError/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: ValueError: PyCapsule_GetPointer called with incorrect name
Run `reticulate::py_last_error()` for details.
Backtrace:1. └─arrow::flight_put(client, example_data, path = flight_obj, max_chunksize = 1) at test-python-flight.R:42:5
 2.   ├─reticulate::r_to_py(data) at r/R/flight.R:73:3
 3.   └─arrow:::r_to_py.Table(data)
 4.     ├─reticulate::r_to_py(as_record_batch_reader(x), convert = FALSE) at r/R/python.R:111:3
 5.     └─arrow:::r_to_py.RecordBatchReader(...)
 6.       └─pa$lib$RecordBatchReader$`_import_from_c`(pyarrow_compatible_pointer(stream_ptr)) at r/R/python.R:227:3
 7.         └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)

── Error (test-python-flight.R:56:5): flight_get ───────────────────────────────
<pyarrow.lib.ArrowKeyError/python.builtin.KeyError/python.builtin.LookupError/pyarrow.lib.ArrowException/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: pyarrow.lib.ArrowKeyError: 'Flight not found.'. Detail: Python exception: KeyError
Run `reticulate::py_last_error()` for details.
Backtrace:1. ├─expect_equal_data_frame(flight_get(client, flight_obj), example_data) at test-python-flight.R:56:5
 2. │ ├─expect_equal(as.data.frame(x), as.data.frame(y), ...) at tests/testthat/helper-expectation.R:24:3
 3. │ │ └─base::inherits(object, "ArrowObject") at tests/testthat/helper-expectation.R:35:3
 4. │ └─base::as.data.frame(x)
 5. └─arrow::flight_get(client, flight_obj)
 6.   └─arrow:::flight_reader(client, path) at r/R/flight.R:96:3
 7.     └─client$get_flight_info(descriptor_for_path(path)) at r/R/flight.R:102:3
 8.       └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)

── Error (test-python-flight.R:61:5): flight_put with RecordBatch ──────────────
<python.builtin.ValueError/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: ValueError: PyCapsule_GetPointer called with incorrect name
Run `reticulate::py_last_error()` for details.
Backtrace:1. └─arrow::flight_put(client, RecordBatch$create(example_data), path = flight_obj2) at test-python-flight.R:61:5
 2.   ├─reticulate::r_to_py(data) at r/R/flight.R:73:3
 3.   └─arrow:::r_to_py.RecordBatch(data)
 4.     └─pa$RecordBatch$`_import_from_c`(...) at r/R/python.R:84:3
 5.       └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)

── Error (test-python-flight.R:66:5): flight_put with overwrite = FALSE ────────
<python.builtin.ValueError/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_call_impl(callable, call_args$unnamed, call_args$named)`: ValueError: PyCapsule_GetPointer called with incorrect name
Run `reticulate::py_last_error()` for details.
Backtrace:1. ├─testthat::expect_error(...) at test-python-flight.R:66:5
  2. │ └─testthat:::expect_condition_matching(...)
  3. │   └─testthat:::quasi_capture(...)
  4. │     ├─testthat (local) .capture(...)
  5. │     │ └─base::withCallingHandlers(...)
  6. │     └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
  7. └─arrow::flight_put(...)
  8.   ├─reticulate::r_to_py(data) at r/R/flight.R:73:3
  9.   └─arrow:::r_to_py.Table(data)
 10.     ├─reticulate::r_to_py(as_record_batch_reader(x), convert = FALSE) at r/R/python.R:111:3
 11.     └─arrow:::r_to_py.RecordBatchReader(...)
 12.       └─pa$lib$RecordBatchReader$`_import_from_c`(pyarrow_compatible_pointer(stream_ptr)) at r/R/python.R:227:3
 13.         └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named)

── Skipped tests (1) ───────────────────────────────────────────────────────────────────────────────────────────────
• empty test (1): test-python-flight.R:75:3

[ FAIL 5 | WARN 0 | SKIP 1 | PASS 2 ]

Test complete

@paleolimbot
Copy link
Member

I believe reticulate 1.35.0 changed the "name" of the capsule. I am not sure what it was before and I am not sure what it is now...we started using external pointers on the R side in Arrow 7.0.0, which converted to Python capsules automatically, because it felt better than converting the pointer to an R double(), sending that to Python, then static casting that to a uintptr_t and calling it a pointer again.

One of the errors is along the lines of PyCapsule_GetPointer() called with incorrect name, so perhaps the name check was made stricter on the Python side when capsules started being used there.

On the R side, we could go back to calling _import_from_c with a floating-point value (although I would rather not). We also have the ability to call it with a string representation of the address, which is also lossless, but this is rejected by pyarrow (I just checked). I also just checked passing a floating point value and that results in the test passing for me.

The code on the R side that could change is here:

arrow/r/R/python.R

Lines 341 to 353 in 672238f

pyarrow_compatible_pointer <- function(ptr) {
pa <- reticulate::import("pyarrow")
version_string <- pa$`__version__`
# remove trailing .devXXX because it won't work with package_version()
pyarrow_version <- package_version(gsub("\\.dev.*?$", "", version_string))
# pyarrow pointers changed in version 7.0.0
if (pyarrow_version >= "7.0.0") {
return(ptr)
} else {
return(external_pointer_addr_double(ptr))
}
}

@amoeba
Copy link
Member

amoeba commented Feb 6, 2024

Thanks @paleolimbot. I just checked the diff between 1.34.0 and 1.35.0 and I think you're right on both counts. In rstudio/reticulate@9b2c8d4 the name changed from "NULL" to "r_extptr" and code was added to validate against the name (by calling PyCapsule_IsValid).

@jorisvandenbossche
Copy link
Member

so perhaps the name check was made stricter on the Python side when capsules started being used there.

FWIW, the python.R code is not yet using capsules (it's still using _import_from_c(..) with raw pointers, as can also be seen in the error traceback), so this "name check" is happening somewhere in reticulate itself?
(just want to say that AFAIU this is not related to the capsule work on the python side)

@paleolimbot
Copy link
Member

Got it!

It looks like capsules with a null name are accepted...perhaps it should be updated to accept a capsule with NULL or r_extptr?

Integers are accepted as well as capsule objects with a NULL name.
(the latter for compatibility with raw pointers exported by reticulate)

@paleolimbot
Copy link
Member

Or, alternatively, we could fix in R by generating the Python integer there:

py <- reticulate::import_builtins(convert = FALSE)
# addr <- external_pointer_addr_double(ptr)
addr <- "12345"
class(py$int(addr))
#> [1] "python.builtin.int"    "python.builtin.object"
py$int(addr)
#> 12345

Created on 2024-02-06 with reprex v2.0.2

@pitrou
Copy link
Member

pitrou commented Feb 6, 2024

In rstudio/reticulate@9b2c8d4 the name changed from "NULL" to "r_extptr" and code was added to validate against the name (by calling PyCapsule_IsValid).

This seems to be about storing R objects inside capsules. This is not what the R-PyArrow bridge is doing, is it?

Also, if the name is not r_extptr, this code doesn't seem to actually change anything.

@jorisvandenbossche
Copy link
Member

My understanding is that the r_to_py will convert any extptr to such a capsule:

https://github.com/rstudio/reticulate/blob/950169382a92328bde39bff3b68f5014ac582986/src/python.cpp#L1971-L1974

And I suppose the array/schema pointers in the R package are "external pointers", and so reticulate automatically puts them in a capsule before passing to _import_from_c (since this is a python call, it converts any argument to it with r_to_py?)

@pitrou
Copy link
Member

pitrou commented Feb 6, 2024

Ahah, I see. Then I guess it makes sense to make the check more tolerant on the PyArrow side.

@paleolimbot
Copy link
Member

I will also submit a fix on the R side...it didn't occur to me at the time to convert to a Python integer using reticulate to work around the fact that R doesn't have 64-bit integers. If it's easy, it would probably also be a good idea to fix it in Python (or remove the ability to pass a pointer in that way, or fix the comment to reflect the new nature of how an external pointer gets converted using reticulate).

@pitrou
Copy link
Member

pitrou commented Feb 6, 2024

We should do both, because ideally one can use an older R-Arrow with a newer PyArrow and vice-versa.

paleolimbot added a commit that referenced this issue Feb 7, 2024
…39969)

### Rationale for this change

The integration tests and documentation build is failing

### What changes are included in this PR?

Instead of relying on how reticulate converts an R external pointer, use a Python integer instead. We can't use an R integer (because they're only 32 bits); we can't use an R double (because the static cast to/from uintptr_t is a bit iffy); however, we can use Python to convert a string to Python integer. This is probably how I should have written it the first time but it didn't occur to me at the time.

### Are these changes tested?

Yes, covered by existing tests.

### Are there any user-facing changes?

No
* Closes: #39933

Lead-authored-by: Dewey Dunnington <[email protected]>
Co-authored-by: Dewey Dunnington <[email protected]>
Signed-off-by: Dewey Dunnington <[email protected]>
@paleolimbot paleolimbot added this to the 16.0.0 milestone Feb 7, 2024
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…ulate (apache#39969)

### Rationale for this change

The integration tests and documentation build is failing

### What changes are included in this PR?

Instead of relying on how reticulate converts an R external pointer, use a Python integer instead. We can't use an R integer (because they're only 32 bits); we can't use an R double (because the static cast to/from uintptr_t is a bit iffy); however, we can use Python to convert a string to Python integer. This is probably how I should have written it the first time but it didn't occur to me at the time.

### Are these changes tested?

Yes, covered by existing tests.

### Are there any user-facing changes?

No
* Closes: apache#39933

Lead-authored-by: Dewey Dunnington <[email protected]>
Co-authored-by: Dewey Dunnington <[email protected]>
Signed-off-by: Dewey Dunnington <[email protected]>
@assignUser assignUser modified the milestones: 16.0.0, 15.0.1 Feb 21, 2024
raulcd pushed a commit that referenced this issue Feb 22, 2024
…39969)

### Rationale for this change

The integration tests and documentation build is failing

### What changes are included in this PR?

Instead of relying on how reticulate converts an R external pointer, use a Python integer instead. We can't use an R integer (because they're only 32 bits); we can't use an R double (because the static cast to/from uintptr_t is a bit iffy); however, we can use Python to convert a string to Python integer. This is probably how I should have written it the first time but it didn't occur to me at the time.

### Are these changes tested?

Yes, covered by existing tests.

### Are there any user-facing changes?

No
* Closes: #39933

Lead-authored-by: Dewey Dunnington <[email protected]>
Co-authored-by: Dewey Dunnington <[email protected]>
Signed-off-by: Dewey Dunnington <[email protected]>
raulcd pushed a commit that referenced this issue Feb 22, 2024
…39969)

### Rationale for this change

The integration tests and documentation build is failing

### What changes are included in this PR?

Instead of relying on how reticulate converts an R external pointer, use a Python integer instead. We can't use an R integer (because they're only 32 bits); we can't use an R double (because the static cast to/from uintptr_t is a bit iffy); however, we can use Python to convert a string to Python integer. This is probably how I should have written it the first time but it didn't occur to me at the time.

### Are these changes tested?

Yes, covered by existing tests.

### Are there any user-facing changes?

No
* Closes: #39933

Lead-authored-by: Dewey Dunnington <[email protected]>
Co-authored-by: Dewey Dunnington <[email protected]>
Signed-off-by: Dewey Dunnington <[email protected]>
zanmato1984 pushed a commit to zanmato1984/arrow that referenced this issue Feb 28, 2024
…ulate (apache#39969)

### Rationale for this change

The integration tests and documentation build is failing

### What changes are included in this PR?

Instead of relying on how reticulate converts an R external pointer, use a Python integer instead. We can't use an R integer (because they're only 32 bits); we can't use an R double (because the static cast to/from uintptr_t is a bit iffy); however, we can use Python to convert a string to Python integer. This is probably how I should have written it the first time but it didn't occur to me at the time.

### Are these changes tested?

Yes, covered by existing tests.

### Are there any user-facing changes?

No
* Closes: apache#39933

Lead-authored-by: Dewey Dunnington <[email protected]>
Co-authored-by: Dewey Dunnington <[email protected]>
Signed-off-by: Dewey Dunnington <[email protected]>
thisisnic pushed a commit that referenced this issue Mar 1, 2024
…39969)

### Rationale for this change

The integration tests and documentation build is failing

### What changes are included in this PR?

Instead of relying on how reticulate converts an R external pointer, use a Python integer instead. We can't use an R integer (because they're only 32 bits); we can't use an R double (because the static cast to/from uintptr_t is a bit iffy); however, we can use Python to convert a string to Python integer. This is probably how I should have written it the first time but it didn't occur to me at the time.

### Are these changes tested?

Yes, covered by existing tests.

### Are there any user-facing changes?

No
* Closes: #39933

Lead-authored-by: Dewey Dunnington <[email protected]>
Co-authored-by: Dewey Dunnington <[email protected]>
Signed-off-by: Dewey Dunnington <[email protected]>
thisisnic pushed a commit to thisisnic/arrow that referenced this issue Mar 8, 2024
…ulate (apache#39969)

### Rationale for this change

The integration tests and documentation build is failing

### What changes are included in this PR?

Instead of relying on how reticulate converts an R external pointer, use a Python integer instead. We can't use an R integer (because they're only 32 bits); we can't use an R double (because the static cast to/from uintptr_t is a bit iffy); however, we can use Python to convert a string to Python integer. This is probably how I should have written it the first time but it didn't occur to me at the time.

### Are these changes tested?

Yes, covered by existing tests.

### Are there any user-facing changes?

No
* Closes: apache#39933

Lead-authored-by: Dewey Dunnington <[email protected]>
Co-authored-by: Dewey Dunnington <[email protected]>
Signed-off-by: Dewey Dunnington <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment