-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
GH-43680: [Integration] Unskip nanoarrow in IPC integration tests #43715
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there are a lot of nanoarrow-related failures in the Integration build
@@ -1876,7 +1876,8 @@ def _temp_path(): | |||
|
|||
generate_null_case([10, 0]), | |||
|
|||
generate_null_trivial_case([0, 0]), | |||
generate_null_trivial_case([0, 0]) | |||
.skip_tester('nanoarrow'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add issue references for these skips?
8cce89d
to
94dd868
Compare
Just a few notes from some investigating:
--- a/dev/archery/archery/integration/tester_nanoarrow.py
+++ b/dev/archery/archery/integration/tester_nanoarrow.py
@@ -56,8 +56,8 @@ class NanoarrowTester(Tester):
'JSON_PATH': json_path,
'COMMAND': command,
**{
- f'QUIRK_{q}': 1
- for q in quirks or []
+ f'QUIRK_{q}': "1"
+ for q in quirks or ""
},
|
|
de12a7a
to
b855bb3
Compare
Rationale for this change
Nanoarrow can now read and write IPC files as of apache/arrow-nanoarrow#585 so it should no longer be skipped as a producer/consumer
What changes are included in this PR?
Nanoarrow's tester is updated to point to the new integration executable and to report nanoarrow as a consumer/producer of IPC files.
Notably the
null_trivial
case is skipped even though nanoarrow nominally supports it since it represents a corner case in which nanoarrow's flatbuffers library will not accept some vectors produced by other flatbuffers libraries dvidelabs/flatcc#287Are these changes tested?
Yes
Are there any user-facing changes?
No