Skip to content

Commit

Permalink
fix future calculation in groupby dynamic (#2935)
Browse files Browse the repository at this point in the history
  • Loading branch information
ritchie46 committed Mar 20, 2022
1 parent 2b80e6b commit 7c743bf
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 17 deletions.
1 change: 1 addition & 0 deletions polars/polars-time/src/groupby/mod.rs
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
#[cfg(any(feature = "dtype-date", feature = "dtype-datetime"))]
pub(crate) mod dynamic;
8 changes: 5 additions & 3 deletions polars/polars-time/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ mod truncate;
mod upsample;
mod windows;

#[cfg(any(feature = "dtype-date", feature = "dtype-datetime"))]
pub use groupby::dynamic::*;

pub use {
date_range::*, groupby::dynamic::*, truncate::*, upsample::*,
windows::calendar::date_range as date_range_vec, windows::duration::Duration,
windows::groupby::ClosedWindow, windows::window::Window,
date_range::*, truncate::*, upsample::*, windows::calendar::date_range as date_range_vec,
windows::duration::Duration, windows::groupby::ClosedWindow, windows::window::Window,
};
22 changes: 9 additions & 13 deletions polars/polars-time/src/windows/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pub struct Bounds {

impl Bounds {
/// Create a new `Bounds` and check the input is correct.
pub fn new_checked(start: i64, stop: i64) -> Self {
pub(crate) fn new_checked(start: i64, stop: i64) -> Self {
assert!(
start <= stop,
"boundary start must be smaller than stop; is your time column sorted in ascending order?\
Expand All @@ -18,21 +18,17 @@ impl Bounds {
}

/// Create a new `Bounds` without checking input correctness.
pub fn new(start: i64, stop: i64) -> Self {
pub(crate) fn new(start: i64, stop: i64) -> Self {
Bounds { start, stop }
}

/// Duration in unit for this Boundary
pub fn duration(&self) -> i64 {
pub(crate) fn duration(&self) -> i64 {
self.stop - self.start
}

pub fn is_empty(&self) -> bool {
self.stop == self.start
}

// check if unit is within bounds
pub fn is_member(&self, t: i64, closed: ClosedWindow) -> bool {
pub(crate) fn is_member(&self, t: i64, closed: ClosedWindow) -> bool {
match closed {
ClosedWindow::Right => t > self.start && t <= self.stop,
ClosedWindow::Left => t >= self.start && t < self.stop,
Expand All @@ -41,10 +37,10 @@ impl Bounds {
}
}

pub fn is_future(&self, t: i64) -> bool {
t > self.stop
}
pub fn is_past(&self, t: i64) -> bool {
t < self.start
pub(crate) fn is_future(&self, t: i64, closed: ClosedWindow) -> bool {
match closed {
ClosedWindow::Left | ClosedWindow::None => self.stop <= t,
ClosedWindow::Both | ClosedWindow::Right => t > self.stop,
}
}
}
4 changes: 3 additions & 1 deletion polars/polars-time/src/windows/groupby.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub fn groupby_windows(
// find starting point of window
while start_offset < time.len() {
let t = time[start_offset];
if bi.is_future(t) {
if bi.is_future(t, closed_window) {
// the window is behind the time values.
skip_window = true;
break;
Expand Down Expand Up @@ -105,6 +105,8 @@ pub fn groupby_windows(
}

let first = i as IdxSize;
dbg!(start_offset);
dbg!(first);

while i < time.len() {
let t = time[i];
Expand Down
19 changes: 19 additions & 0 deletions polars/polars-time/src/windows/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,3 +468,22 @@ fn test_groupby_windows_membership_2791() {
assert_eq!(groups[0], [0, 2]);
assert_eq!(groups[1], [2, 2]);
}

#[test]
fn test_groupby_windows_duplicates_2931() {
let dates = [0, 3, 3, 5, 5];
let window = Window::new(
Duration::parse("1ms"),
Duration::parse("1ms"),
Duration::parse("0ns"),
);

let (groups, _, _) = groupby_windows(
window,
&dates,
false,
ClosedWindow::Left,
TimeUnit::Milliseconds,
);
assert_eq!(groups, [[0, 1], [1, 2], [3, 2]]);
}

0 comments on commit 7c743bf

Please sign in to comment.