kou commented on a change in pull request #11167:
URL: https://github.com/apache/arrow/pull/11167#discussion_r711353082
##########
File path: ruby/red-arrow/test/test-table.rb
##########
@@ -147,6 +147,96 @@ def setup
TABLE
end
+ test("{key: Number}") do
+ assert_equal(<<-TABLE, @table.slice(count: 16).to_s)
+ count visible
+0 16 true
+ TABLE
+ end
+
+ test("{key: String}") do
+ name_array = Arrow::StringArray.new(["a", "b", "c", "d", "e", "f", "g",
"h"])
+ table_with_name = @table.merge(:name => name_array)
+ assert_equal(<<-TABLE, table_with_name.slice(name: 'e').to_s)
+ count visible name
+0 16 true e
+ TABLE
+ end
+
+ test("{key: TrueClass}") do
+ assert_equal(<<-TABLE, @table.slice(visible: true).to_s)
+ count visible
+0 1 true
+1 (null) (null)
+2 8 true
+3 16 true
+4 (null) (null)
+5 (null) (null)
+ TABLE
+ end
+
+ test("{key: FalseClass}") do
+ assert_equal(<<-TABLE, @table.slice(visible: false).to_s)
+ count visible
+0 2 false
+1 (null) (null)
+2 32 false
+3 (null) (null)
+4 (null) (null)
+ TABLE
+ end
+
+ test("{key: Range}: beginless") do
+ assert_equal(<<-TABLE, @table.slice(count: ..8).to_s)
+ count visible
+0 1 true
+1 2 false
+2 4 (null)
+3 8 true
+ TABLE
+ end
+
+ test("{key: Range}: endless") do
+ assert_equal(<<-TABLE, @table.slice(count: 16..).to_s)
+ count visible
+0 16 true
+1 32 false
+2 64 (null)
+3 128 (null)
+ TABLE
+ end
+
+ test("{key: Range}: include end") do
+ assert_equal(<<-TABLE, @table.slice(count: 1..16).to_s)
+ count visible
+0 1 true
+1 2 false
+2 4 (null)
+3 8 true
+4 16 true
+ TABLE
+ end
+
+ test("{key: Range}: exclude end") do
+ assert_equal(<<-TABLE, @table.slice(count: 1...16).to_s)
+ count visible
+0 1 true
+1 2 false
+2 4 (null)
+3 8 true
+ TABLE
+ end
+
+ test("{key: Range, key: TrueClass}: multiple key/value pairs") do
Review comment:
We can simplify this because we can show this is multiple key/value
pairs case by `{key1: ..., key2: ...}`:
```suggestion
test("{key1: Range, key2: true}") do
```
##########
File path: ruby/red-arrow/test/test-table.rb
##########
@@ -147,6 +147,96 @@ def setup
TABLE
end
+ test("{key: Number}") do
+ assert_equal(<<-TABLE, @table.slice(count: 16).to_s)
+ count visible
+0 16 true
+ TABLE
+ end
+
+ test("{key: String}") do
+ name_array = Arrow::StringArray.new(["a", "b", "c", "d", "e", "f", "g",
"h"])
+ table_with_name = @table.merge(:name => name_array)
Review comment:
Or we can use a table that only has `name`:
```ruby
table = Arrow::Table.new(name: Arrow::StringArray.new(["a", "b", "c"]))
assert_equal(..., table.slice(name: "b").to_s)
...
```
##########
File path: ruby/red-arrow/test/test-table.rb
##########
@@ -147,6 +147,96 @@ def setup
TABLE
end
+ test("{key: Number}") do
+ assert_equal(<<-TABLE, @table.slice(count: 16).to_s)
+ count visible
+0 16 true
+ TABLE
+ end
+
+ test("{key: String}") do
+ name_array = Arrow::StringArray.new(["a", "b", "c", "d", "e", "f", "g",
"h"])
+ table_with_name = @table.merge(:name => name_array)
+ assert_equal(<<-TABLE, table_with_name.slice(name: 'e').to_s)
+ count visible name
+0 16 true e
+ TABLE
+ end
+
+ test("{key: TrueClass}") do
Review comment:
We can simplify this because `true` is the only one instance of
`TrueClass`:
```suggestion
test("{key: true}") do
```
##########
File path: ruby/red-arrow/test/test-table.rb
##########
@@ -147,6 +147,96 @@ def setup
TABLE
end
+ test("{key: Number}") do
+ assert_equal(<<-TABLE, @table.slice(count: 16).to_s)
+ count visible
+0 16 true
+ TABLE
+ end
+
+ test("{key: String}") do
+ name_array = Arrow::StringArray.new(["a", "b", "c", "d", "e", "f", "g",
"h"])
+ table_with_name = @table.merge(:name => name_array)
Review comment:
```suggestion
table_with_name = @table.merge(name: name_array)
```
##########
File path: ruby/red-arrow/test/test-table.rb
##########
@@ -147,6 +147,96 @@ def setup
TABLE
end
+ test("{key: Number}") do
+ assert_equal(<<-TABLE, @table.slice(count: 16).to_s)
+ count visible
+0 16 true
+ TABLE
+ end
+
+ test("{key: String}") do
+ name_array = Arrow::StringArray.new(["a", "b", "c", "d", "e", "f", "g",
"h"])
+ table_with_name = @table.merge(:name => name_array)
+ assert_equal(<<-TABLE, table_with_name.slice(name: 'e').to_s)
+ count visible name
+0 16 true e
+ TABLE
+ end
+
+ test("{key: TrueClass}") do
+ assert_equal(<<-TABLE, @table.slice(visible: true).to_s)
+ count visible
+0 1 true
+1 (null) (null)
+2 8 true
+3 16 true
+4 (null) (null)
+5 (null) (null)
+ TABLE
+ end
+
+ test("{key: FalseClass}") do
+ assert_equal(<<-TABLE, @table.slice(visible: false).to_s)
+ count visible
+0 2 false
+1 (null) (null)
+2 32 false
+3 (null) (null)
+4 (null) (null)
+ TABLE
+ end
+
+ test("{key: Range}: beginless") do
+ assert_equal(<<-TABLE, @table.slice(count: ..8).to_s)
+ count visible
+0 1 true
+1 2 false
+2 4 (null)
+3 8 true
+ TABLE
+ end
Review comment:
Could you also add begeinless and excluded end range case? (`...8`)
##########
File path: ruby/red-arrow/test/test-table.rb
##########
@@ -147,6 +147,96 @@ def setup
TABLE
end
+ test("{key: Number}") do
+ assert_equal(<<-TABLE, @table.slice(count: 16).to_s)
+ count visible
+0 16 true
+ TABLE
+ end
+
+ test("{key: String}") do
+ name_array = Arrow::StringArray.new(["a", "b", "c", "d", "e", "f", "g",
"h"])
+ table_with_name = @table.merge(:name => name_array)
+ assert_equal(<<-TABLE, table_with_name.slice(name: 'e').to_s)
+ count visible name
+0 16 true e
+ TABLE
+ end
+
+ test("{key: TrueClass}") do
+ assert_equal(<<-TABLE, @table.slice(visible: true).to_s)
+ count visible
+0 1 true
+1 (null) (null)
+2 8 true
+3 16 true
+4 (null) (null)
+5 (null) (null)
+ TABLE
+ end
+
+ test("{key: FalseClass}") do
Review comment:
ditto.
##########
File path: ruby/red-arrow/test/test-table.rb
##########
@@ -147,6 +147,96 @@ def setup
TABLE
end
+ test("{key: Number}") do
+ assert_equal(<<-TABLE, @table.slice(count: 16).to_s)
+ count visible
+0 16 true
+ TABLE
+ end
+
+ test("{key: String}") do
+ name_array = Arrow::StringArray.new(["a", "b", "c", "d", "e", "f", "g",
"h"])
+ table_with_name = @table.merge(:name => name_array)
+ assert_equal(<<-TABLE, table_with_name.slice(name: 'e').to_s)
+ count visible name
+0 16 true e
+ TABLE
+ end
+
+ test("{key: TrueClass}") do
+ assert_equal(<<-TABLE, @table.slice(visible: true).to_s)
+ count visible
+0 1 true
+1 (null) (null)
+2 8 true
+3 16 true
+4 (null) (null)
+5 (null) (null)
+ TABLE
+ end
+
+ test("{key: FalseClass}") do
+ assert_equal(<<-TABLE, @table.slice(visible: false).to_s)
+ count visible
+0 2 false
+1 (null) (null)
+2 32 false
+3 (null) (null)
+4 (null) (null)
+ TABLE
+ end
+
+ test("{key: Range}: beginless") do
+ assert_equal(<<-TABLE, @table.slice(count: ..8).to_s)
+ count visible
+0 1 true
+1 2 false
+2 4 (null)
+3 8 true
+ TABLE
+ end
+
+ test("{key: Range}: endless") do
+ assert_equal(<<-TABLE, @table.slice(count: 16..).to_s)
+ count visible
+0 16 true
+1 32 false
+2 64 (null)
+3 128 (null)
+ TABLE
+ end
+
+ test("{key: Range}: include end") do
+ assert_equal(<<-TABLE, @table.slice(count: 1..16).to_s)
+ count visible
+0 1 true
+1 2 false
+2 4 (null)
+3 8 true
+4 16 true
+ TABLE
+ end
+
+ test("{key: Range}: exclude end") do
+ assert_equal(<<-TABLE, @table.slice(count: 1...16).to_s)
+ count visible
+0 1 true
+1 2 false
+2 4 (null)
+3 8 true
+ TABLE
+ end
+
+ test("{key: Range, key: TrueClass}: multiple key/value pairs") do
+ assert_equal(<<-TABLE, @table.slice(count: 0..8, visible: false).to_s)
+ count visible
+0 2 false
+1 (null) (null)
+2 (null) (null)
+3 (null) (null)
+ TABLE
Review comment:
This expected result looks wrong...
It should be:
```text
count visible
0 2 false
```
(We should remove null records from `==` result.)
But this isn't a problem in this pull request. We can fix it in another pull
request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
