[GitHub] [arrow] alamb edited a comment on pull request #8985: ARROW-11005: [Rust] Remove indirection from `take` kernel

[GitBox <gi...@apache.org>]

alamb edited a comment on pull request #8985:
URL: https://github.com/apache/arrow/pull/8985#issuecomment-749513687


   > The rational for this PR is that it makes it more obvious the design of the module: each array type has its own implementation (e.g. take_primitive<T>(array: &PrimitiveArray<T>, indices: ...) -> PrimitiveArray<T>), and there is one dyn implementation, take(array: &Array, indices: ...) -> ArrayRef, that uses each type-specific implementation and wraps it on an Arc<Array> for the dyn behavior. 
   
   I think this is a great design and perhaps we should mirror it in other kernels (e.g. https://github.com/apache/arrow/blob/master/rust/arrow/src/compute/kernels/aggregate.rs)
   
   @jorgecarleitao  and @nevi-me  if you agree, I can try and write up some tickets for adding such apis to the different aggregate modules. I think that would be good introductory PRs for people who wanted to contribute and it would make the library easier to use
   
   UPDATE: though now upon more thought aggregate would need some additional finagling as the aggregate types that come out are different. Maybe modules like `comparison` (e.g. `eq`) would be more amenable to standardization


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org