Conversation
Member
Author
|
We should rebase once #49710 is merged as changes from that PR are in this branch as they were needed to make it work. |
thisisnic
force-pushed
the
GH-45523-ipc-polars
branch
2 times, most recently
from
May 5, 2026 16:41
e8768d0 to
bb711a7
Compare
thisisnic
commented
May 6, 2026
thisisnic
commented
May 6, 2026
Member
Author
|
A lot of |
thisisnic
force-pushed
the
GH-45523-ipc-polars
branch
from
May 28, 2026 09:37
de59365 to
cf9ad10
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds R bindings and conversion support for Arrow utf8_view / string_view, addressing IPC/table conversion failures for data containing StringView columns.
Changes:
- Adds
string_view()R data type binding, export registration, and type tests. - Implements R↔Arrow conversion paths for StringView arrays and dictionary values.
- Updates generated documentation and related converter support in C++/Python.
Reviewed changes
Copilot reviewed 13 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| r/R/type.R | Adds StringView R6 type, constructor, and canonical type aliases. |
| r/R/arrowExports.R | Registers R wrapper for StringView__initialize. |
| r/NAMESPACE | Exports string_view. |
| r/src/datatype.cpp | Maps Arrow STRING_VIEW to R StringView and initializes utf8_view. |
| r/src/r_to_arrow.cpp | Adds R-to-Arrow StringView conversion and dictionary StringView value handling. |
| r/src/array_to_vector.cpp | Adds Arrow-to-R StringView and wider dictionary index conversion support. |
| r/src/arrowExports.cpp | Registers native StringView initialization entry point. |
| cpp/src/arrow/util/converter.h | Enables dictionary converters for StringViewType. |
| python/pyarrow/src/arrow/python/python_to_arrow.cc | Adjusts Python dictionary StringView append call. |
| r/tests/testthat/test-Array.R | Adds StringView array round-trip tests. |
| r/tests/testthat/test-Table.R | Adds table/dictionary StringView and wider index tests. |
| r/tests/testthat/test-data-type.R | Adds StringView data type and code round-trip tests. |
| r/DESCRIPTION | Updates roxygen metadata. |
| r/man/data-type.Rd | Adds string_view() documentation entry. |
| r/man/acero.Rd | Regenerated Acero documentation. |
| r/man/arrow-package.Rd | Regenerated package author documentation. |
| r/man/csv_convert_options.Rd | Regenerated CSV conversion docs. |
| r/man/csv_read_options.Rd | Regenerated CSV read docs. |
| r/man/CsvReadOptions.Rd | Regenerated CSV read options docs. |
| r/man/enums.Rd | Regenerated enum documentation. |
| r/man/JsonFileFormat.Rd | Regenerated JSON file format docs. |
| r/man/reexports.Rd | Regenerated reexports docs. |
| r/man/vctrs_extension_array.Rd | Regenerated vctrs extension docs. |
Files not reviewed (10)
- r/man/CsvReadOptions.Rd: Language not supported
- r/man/JsonFileFormat.Rd: Language not supported
- r/man/acero.Rd: Language not supported
- r/man/arrow-package.Rd: Language not supported
- r/man/csv_convert_options.Rd: Language not supported
- r/man/csv_read_options.Rd: Language not supported
- r/man/data-type.Rd: Language not supported
- r/man/enums.Rd: Language not supported
- r/man/reexports.Rd: Language not supported
- r/man/vctrs_extension_array.Rd: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 23 changed files in this pull request and generated 1 comment.
Files not reviewed (10)
- r/man/CsvReadOptions.Rd: Language not supported
- r/man/JsonFileFormat.Rd: Language not supported
- r/man/acero.Rd: Language not supported
- r/man/arrow-package.Rd: Language not supported
- r/man/csv_convert_options.Rd: Language not supported
- r/man/csv_read_options.Rd: Language not supported
- r/man/data-type.Rd: Language not supported
- r/man/enums.Rd: Language not supported
- r/man/reexports.Rd: Language not supported
- r/man/vctrs_extension_array.Rd: Language not supported
thisisnic
force-pushed
the
GH-45523-ipc-polars
branch
from
June 10, 2026 15:10
a74c9c1 to
1842e7e
Compare
Member
Author
|
Current CI failure is unrelated; #50219 |
pitrou
reviewed
Jul 29, 2026
thisisnic
force-pushed
the
GH-45523-ipc-polars
branch
from
August 18, 2026 13:02
03be67f to
b07d6f9
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
…nary The offset-buffer fast path in Converter_String::Ingest_some_nulls() hardcoded GetValues<int32_t>, which is incorrect for LargeStringArray (int64_t offsets). Use StringArrayType::offset_type so the correct width is resolved at compile time. Also switch the view-type guard from a runtime is_binary_view_like() check to if constexpr, since StringViewArray has no offset_type member and the template would fail to instantiate otherwise. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Exercise out-of-line storage (>12 bytes) in the dictionary string_view test and add NULL + mixed lengths to the binary_view table test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
thisisnic
force-pushed
the
GH-45523-ipc-polars
branch
from
September 4, 2026 10:34
0712aa7 to
8878bfa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
No bindings for Utf8View type in the R package
What changes are included in this PR?
Implement bindings
Are these changes tested?
Yep
Are there any user-facing changes?
Yep, adding functionality.
AI Usage
Heavily used Codex/Claude here. I'm not confident of every line of code. I read things over, and iterated on it making sure that tests pass and nothing seemed wildly incorrect.