Repository navigation
ARROW-17500: [Go] Kernel and KernelContext interfaces - #13946
Conversation
|
|
pitrou
left a comment
pitrou
left a comment
There was a problem hiding this comment.
Do you plan to add any docstrings here?
|
@pitrou I probably should despite these all being internal and not public, so the documentation is clear for implementing kernels. I'll go do that |
|
@pitrou I've added the Godoc comments for the types and functions here. |
|
Is there a Go developer that can give this a review @zeroshade ? |
|
@cyb70289 is really the only one I know who regularly comments (which is why i requested a review from him) Unfortunately there really isn't any other regular Go developers contributing currently 😞 |
|
I'll wait until tomorrow to merge this in order to give @cyb70289 time to comment if desired. |
| } | ||
|
|
||
| func (s sameTypeIDMatcher) Matches(typ arrow.DataType) bool { return s.accepted == typ.ID() } | ||
| func (s *sameTypeIDMatcher) Equals(other TypeMatcher) bool { |
There was a problem hiding this comment.
Nit: a bit dogmatic but is it possible to make this method also accepts value, not pointer?
From https://go.dev/doc/faq#methods_on_values_or_pointers, looks it's recommended to use value or pointer receiver consistently.
Next is consistency. If some of the methods of the type must have pointer receivers, the rest should too, so the method set is consistent regardless of how the type is used. See the section on method sets for details.
There was a problem hiding this comment.
Sure, I'll modify it for that
This implements the interface for Kernels along with a `KernelSignature` struct and the type matching for `InputType` and `OutputType` via a `TypeMatcher` interface. Along with tests for all of the above. Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
|
Benchmark runs are scheduled for baseline = 712e06b and contender = 28b81ec. 28b81ec is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
This implements the interface for Kernels along with a `KernelSignature` struct and the type matching for `InputType` and `OutputType` via a `TypeMatcher` interface. Along with tests for all of the above. Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
This implements the interface for Kernels along with a `KernelSignature` struct and the type matching for `InputType` and `OutputType` via a `TypeMatcher` interface. Along with tests for all of the above. Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
This implements the interface for Kernels along with a `KernelSignature` struct and the type matching for `InputType` and `OutputType` via a `TypeMatcher` interface. Along with tests for all of the above. Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
This implements the interface for Kernels along with a `KernelSignature` struct and the type matching for `InputType` and `OutputType` via a `TypeMatcher` interface. Along with tests for all of the above. Authored-by: Matt Topol <[email protected]> Signed-off-by: Matt Topol <[email protected]>
This implements the interface for Kernels along with a
KernelSignaturestruct and the type matching forInputTypeandOutputTypevia aTypeMatcherinterface. Along with tests for all of the above.