luisquintanilla commented on PR #379: URL: https://github.com/apache/arrow-dotnet/pull/379#issuecomment-5108826488
Hey folks, Got caught up in other things so I didn't have a chance to get back to this. Thanks for the feedback. I've addressed it in the latest commit. ## Change summary ### Return `Nullable<T>` All of the operations now return `Nullable<T>` when there are no non-null elements. This aligns with Arrow's default ([`ScalarAggregateOptions`](https://github.com/apache/arrow/blob/main/cpp/src/arrow/compute/api_aggregate.h) uses `min_count = 1`, "emit null" below that many non-null values) and [PyArrow's `None`](https://arrow.apache.org/docs/python/generated/pyarrow.compute.ScalarAggregateOptions.html) Worth calling out, to remain consistent with Arrow, `Sum` also follow that same pattern now. ### Package Naming / Guidelines Looking again at the other languages, I think the main reasoning for using those names and separate packages is consistency and following of established patterns in the broader Arrow ecosystem. - **Core** - Describes the common data types and what the data is (buffers, schema, arrays, etc) - **Compute** - Derives data from values (aggregate, sort, filter, arithmetic, etc.) Naming aside, one other point to support the split, `Compute` adds the `System.Numerics.Tensors` dependency. While lightweight, not everyone may want to have that dependency in the box when they consume `Core` especially when they're not utilizing the *compute* operations. cc: @CurtHagenlocher @adamreeve for visibility. -- 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]
