-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Parquet: Update Variant reader test to generate shared test cases. #13654
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
Conversation
21ddf9d to
e29f1e7
Compare
| GenericRecord var1 = | ||
| record( | ||
| variantType, Map.of("metadata", VariantTestUtil.emptyMetadata(), "typed_value", arr1)); | ||
| GenericRecord row1 = record(parquetSchema, Map.of("id", 1, "var", var1)); |
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.
For consistency with other test cases, I changed the row IDs to match the row position in this test case.
anoopj
left a comment
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.
LGTM
| Record record = records.get(i); | ||
| String variantFile = variantFiles.get(i); | ||
|
|
||
| if (variantFile != null) { |
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.
Nit: for readability, consider flipping the order:
if (variantFile == null) {
// null logic here
} else {
// non-null logic here.
}
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
This PR updates the reader tests for Parquet so that the Parquet files and matching variants can be saved for cross-language tests.