-
Notifications
You must be signed in to change notification settings - Fork 3.9k
GH-48362: [GLib][Ruby] Add FixedSizeListArray #48369
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
|
|
kou
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.
+1
| def setup | ||
| require_gi_bindings(3, 1, 9) | ||
| end | ||
|
|
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.
Could you define a helper method that gets the i-th value to shorten test code?
| def get_value(array, i) | |
| value = array.get_value(i) | |
| value.length.times.collect do |j| | |
| value.get_value(j) | |
| end | |
| end | |
| assert_equal([1, 2], array.get_value(0).length.times.collect {|i| | ||
| array.get_value(0).get_value(i)}) | ||
| assert_equal([3, 4], array.get_value(1).length.times.collect {|i| | ||
| array.get_value(1).get_value(i)}) |
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.
| assert_equal([1, 2], array.get_value(0).length.times.collect {|i| | |
| array.get_value(0).get_value(i)}) | |
| assert_equal([3, 4], array.get_value(1).length.times.collect {|i| | |
| array.get_value(1).get_value(i)}) | |
| assert_equal([1, 2], get_value(array, 0)) | |
| assert_equal([3, 4], get_value(array, 1)) |
| assert_equal([1, 2], array.get_value(0).length.times.collect {|i| | ||
| array.get_value(0).get_value(i)}) |
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.
| assert_equal([1, 2], array.get_value(0).length.times.collect {|i| | |
| array.get_value(0).get_value(i)}) | |
| assert_equal([1, 2], get_value(array, 0)) |
| assert_equal(3, array.length) | ||
| assert_equal([1, 2], array.get_value(0).length.times.collect {|i| | ||
| array.get_value(0).get_value(i)}) | ||
| assert_equal(true, array.null?(1)) |
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.
Could you use assert with block for predicate? It shows more useful information on failure.
| assert_equal(true, array.null?(1)) | |
| assert do | |
| array.null?(1) | |
| end |
| assert_equal([5, 6], array.get_value(2).length.times.collect {|i| | ||
| array.get_value(2).get_value(i)}) |
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.
| assert_equal([5, 6], array.get_value(2).length.times.collect {|i| | |
| array.get_value(2).get_value(i)}) | |
| assert_equal([5, 6], get_value(array, 2)) |
8f7ca5f to
60aff82
Compare
|
Thanks for the review @kou. Updated according to your suggestions. |
Rationale for this change
The
FixedSizeListArrayclass is not available in GLib/Ruby, and it can be the result of e.g. theextract_regex_spanandlist_slicecompute functions.What changes are included in this PR?
This adds the
GArrowFixedSizeListArrayandGArrowFixedSizeListArrayBuilderto GLib.Are these changes tested?
Yes, with Ruby unit tests.
Are there any user-facing changes?
Yes, new classes.