implement eq_dyn, neq_dyn, lt_dyn, lt_eq_dyn, gt_dyn, gt_eq_dyn
#858
Conversation
jimexist
changed the title
eq_dyn, neq_dyn, lt_dyn, lt_eq_dyn, gt_dyn, gt_eq_dyn
jimexist
force-pushed
the
impl-eq-neq-dyn
branch
from
84250ae to
7179a44
Compare
Codecov Report
@@ Coverage Diff @@
## master #858 +/- ##
==========================================
- Coverage 82.66% 82.65% -0.02%
==========================================
Files 168 168
Lines 48172 48196 +24
==========================================
+ Hits 39823 39834 +11
- Misses 8349 8362 +13
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I think this looks really nice @jimexist -- thank you!
FWIW I took a friendly look at what arrow2 does for this situation and I think this PR is not inconsistent. Arrow2 appears to have a function called arithmetic which does the type dispatch on the parameter types and an Operator enum:
https://github.com/jorgecarleitao/arrow2/blob/main/src/compute/arithmetics/mod.rs#L91
While an Operator enum seems like a reasonable thing to consider adding to arrow-rs it is also likely fine to do as a follow on (as we already have different kernels for different operations)
| /// The main reason for this macro is that inputs and outputs align nicely after `cargo fmt`. | ||
| macro_rules! cmp_i64 { | ||
| ($KERNEL:ident, $A_VEC:expr, $B_VEC:expr, $EXPECTED:expr) => { | ||
| ($KERNEL:ident, $DYN_KERNEL:ident, $A_VEC:expr, $B_VEC:expr, $EXPECTED:expr) => { |
jimexist
force-pushed
the
impl-eq-neq-dyn
branch
from
e6c66aa to
08b53e5
Compare
4 participants
Which issue does this PR close?
Closes #843
Rationale for this change
to reduce casting on the call sites
What changes are included in this PR?
implement:
eq_dynneq_dynlt_dynlt_eq_dyngt_dyngt_eq_dynAre there any user-facing changes?