apache / arrow-nanoarrow

Helpers for Arrow C Data & Arrow C Stream interfaces
https://arrow.apache.org/nanoarrow
Apache License 2.0
149 stars 34 forks source link

refactor(r): Use JSON in experimental R vctrs extension type #533

Closed paleolimbot closed 1 week ago

paleolimbot commented 1 week ago

This PR updates the "vctrs extension type" to use JSON as its metadata serialization. JSON is use for most other extension types and using it here provides some workaround for non-R consumers that encounter this type elsewhere.

The serializer and deserializer use the same format as jsonlite::serializeJSON(), but restrict the types of objects that it is willing to serialize or deserialize.

amoeba commented 1 week ago

This looks good @paleolimbot. I am getting one test failure locally though:

══ Results ═══════════════════════════════════════════════════════════════════════════
Duration: 4.6 s

── Failed tests ──────────────────────────────────────────────────────────────────────
Failure (test-extension.R:83:3): as_nanoarrow_array() dispatches on registered extension spec
`as_nanoarrow_array(...)` did not throw the expected error.

[ FAIL 1 | WARN 0 | SKIP 0 | PASS 1426 ]
paleolimbot commented 1 week ago

Thank you for the review!

Is there any chance that the latest commit fixes the issue? (I'm wondering if it's related to the fact that we can't really unregister s3 methods and this may have changed in R 4.4).

amoeba commented 1 week ago

Yep, that did it. All tests pass now.