Compute function test harness #1719
Comments
danking
added a commit
that referenced
this issue
Jan 6, 2025
Lessons learned: do not merge without at least basic tests. Four major changes: 1. Fix arithmetic operations to be correct (vis-a-vis lengths and commuting). 2. Basic test for binary_numeric, parameterized by an array (following the approach described in #1719). 3. Add FlippedSub and FlippedDiv to support checking if the RHS supports the given operation. 4. Ensure the binary numeric implementation is always in `binary_numeric.rs` We check for a constant array but do not create a new constant array with the appropriate length for the child on which we want to delegate the binary numeric operation. That is now fixed. I also added a very basic test suite for applying the (now six) numeric operations on pairs of arrays where one of the two are constant. In order to properly support constant arrays on the left-hand-side, I added FlippedSub and FlippedDiv which allow us to commute/flip the operator so that we need not teach ConstantArray to check if its right-hand-side supports an operation on constants.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We should write a test harness for each compute function. It should ask the encoding for arrays of various dtype, length, nullability etc.
This code can be feature flagged behind a custom
test-harnessfeature, for which there's a funky workaround to allow the same crate to enable its own optional feature as a dev-dependency. See https://users.rust-lang.org/t/how-to-reference-the-test-of-another-crate-in-the-current-crate/91965/4The text was updated successfully, but these errors were encountered: