Feature add weekday temporal kernel #1891
Conversation
nl5887
force-pushed
the
feature-add-weekday-temporal-kernel
branch
from
d1a5f47 to
b510a7d
Compare
Codecov Report
@@ Coverage Diff @@
## master #1891 +/- ##
==========================================
- Coverage 83.48% 83.48% -0.01%
==========================================
Files 201 203 +2
Lines 56838 57156 +318
==========================================
+ Hits 47452 47715 +263
- Misses 9386 9441 +55
Continue to review full report at Codecov.
|
| vec![Some(1514764800000), None, Some(1550636625000)].into(); | ||
|
|
||
| let b = weekday(&a).unwrap(); | ||
| assert_eq!(1, b.value(1)); |
|
Looks like at least one test need to be updated: https://github.com/apache/arrow-rs/runs/6924858466?check_suite_focus=true
|
Thanks, updated the test. Any opinions if we should use the ISO 8601 (similar to chrono-rs) week days (starting at Monday) or the US (starting at Sunday)? Currently it is using ISO (https://docs.rs/chrono/latest/src/chrono/lib.rs.html#646). |
I don't have any particular opinion -- it is likely that whatever we pick will be wrong for some usecases. The best long term is likely to allow the user to specify what behavior they want but I think we can do that in a follow on PR if anyone needs it |
| @@ -211,6 +211,34 @@ where | |||
| Ok(b.finish()) | |||
| } | |||
|
|
|||
| /// Extracts the day of week of a given temporal array as an array of integers | |||
There was a problem hiding this comment.
| /// Extracts the day of week of a given temporal array as an array of integers | |
| /// Extracts the day of week of a given temporal array as an array of integers | |
| /// Monday is 0, Tuesday is 1, etc. |
This feature will add the temporal kernel for weekday (day of week). When added, DOW support can be add to datafusion.