Skip to content

Commit

Permalink
fix: Make Duration parsing fallible and not panic (#19490)
Browse files Browse the repository at this point in the history
  • Loading branch information
coastalwhite authored Oct 28, 2024
1 parent 3cded29 commit 75aa4a3
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 85 deletions.
6 changes: 3 additions & 3 deletions crates/polars-python/src/dataframe/general.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,11 +591,11 @@ impl PyDataFrame {
every: &str,
stable: bool,
) -> PyResult<Self> {
let every = Duration::try_parse(every).map_err(PyPolarsErr::from)?;
let out = if stable {
self.df
.upsample_stable(by, index_column, Duration::parse(every))
self.df.upsample_stable(by, index_column, every)
} else {
self.df.upsample(by, index_column, Duration::parse(every))
self.df.upsample(by, index_column, every)
};
let out = out.map_err(PyPolarsErr::from)?;
Ok(out.into())
Expand Down
18 changes: 10 additions & 8 deletions crates/polars-python/src/expr/general.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use pyo3::class::basic::CompareOp;
use pyo3::prelude::*;

use crate::conversion::{parse_fill_null_strategy, vec_extract_wrapped, Wrap};
use crate::error::PyPolarsErr;
use crate::map::lazy::map_single;
use crate::PyExpr;

Expand Down Expand Up @@ -614,15 +615,15 @@ impl PyExpr {
period: &str,
offset: &str,
closed: Wrap<ClosedWindow>,
) -> Self {
) -> PyResult<Self> {
let options = RollingGroupOptions {
index_column: index_column.into(),
period: Duration::parse(period),
offset: Duration::parse(offset),
period: Duration::try_parse(period).map_err(PyPolarsErr::from)?,
offset: Duration::try_parse(offset).map_err(PyPolarsErr::from)?,
closed_window: closed.0,
};

self.inner.clone().rolling(options).into()
Ok(self.inner.clone().rolling(options).into())
}

fn and_(&self, expr: Self) -> Self {
Expand Down Expand Up @@ -812,12 +813,13 @@ impl PyExpr {
};
self.inner.clone().ewm_mean(options).into()
}
fn ewm_mean_by(&self, times: PyExpr, half_life: &str) -> Self {
let half_life = Duration::parse(half_life);
self.inner
fn ewm_mean_by(&self, times: PyExpr, half_life: &str) -> PyResult<Self> {
let half_life = Duration::try_parse(half_life).map_err(PyPolarsErr::from)?;
Ok(self
.inner
.clone()
.ewm_mean_by(times.inner, half_life)
.into()
.into())
}

fn ewm_std(
Expand Down
55 changes: 29 additions & 26 deletions crates/polars-python/src/expr/rolling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use pyo3::prelude::*;
use pyo3::types::PyFloat;

use crate::conversion::Wrap;
use crate::error::PyPolarsErr;
use crate::map::lazy::call_lambda_with_series;
use crate::{PyExpr, PySeries};

Expand Down Expand Up @@ -34,14 +35,14 @@ impl PyExpr {
window_size: &str,
min_periods: usize,
closed: Wrap<ClosedWindow>,
) -> Self {
) -> PyResult<Self> {
let options = RollingOptionsDynamicWindow {
window_size: Duration::parse(window_size),
window_size: Duration::try_parse(window_size).map_err(PyPolarsErr::from)?,
min_periods,
closed_window: closed.0,
fn_params: None,
};
self.inner.clone().rolling_sum_by(by.inner, options).into()
Ok(self.inner.clone().rolling_sum_by(by.inner, options).into())
}

#[pyo3(signature = (window_size, weights, min_periods, center))]
Expand Down Expand Up @@ -70,14 +71,14 @@ impl PyExpr {
window_size: &str,
min_periods: usize,
closed: Wrap<ClosedWindow>,
) -> Self {
) -> PyResult<Self> {
let options = RollingOptionsDynamicWindow {
window_size: Duration::parse(window_size),
window_size: Duration::try_parse(window_size).map_err(PyPolarsErr::from)?,
min_periods,
closed_window: closed.0,
fn_params: None,
};
self.inner.clone().rolling_min_by(by.inner, options).into()
Ok(self.inner.clone().rolling_min_by(by.inner, options).into())
}

#[pyo3(signature = (window_size, weights, min_periods, center))]
Expand Down Expand Up @@ -105,14 +106,14 @@ impl PyExpr {
window_size: &str,
min_periods: usize,
closed: Wrap<ClosedWindow>,
) -> Self {
) -> PyResult<Self> {
let options = RollingOptionsDynamicWindow {
window_size: Duration::parse(window_size),
window_size: Duration::try_parse(window_size).map_err(PyPolarsErr::from)?,
min_periods,
closed_window: closed.0,
fn_params: None,
};
self.inner.clone().rolling_max_by(by.inner, options).into()
Ok(self.inner.clone().rolling_max_by(by.inner, options).into())
}

#[pyo3(signature = (window_size, weights, min_periods, center))]
Expand Down Expand Up @@ -142,15 +143,15 @@ impl PyExpr {
window_size: &str,
min_periods: usize,
closed: Wrap<ClosedWindow>,
) -> Self {
) -> PyResult<Self> {
let options = RollingOptionsDynamicWindow {
window_size: Duration::parse(window_size),
window_size: Duration::try_parse(window_size).map_err(PyPolarsErr::from)?,
min_periods,
closed_window: closed.0,
fn_params: None,
};

self.inner.clone().rolling_mean_by(by.inner, options).into()
Ok(self.inner.clone().rolling_mean_by(by.inner, options).into())
}

#[pyo3(signature = (window_size, weights, min_periods, center, ddof))]
Expand Down Expand Up @@ -182,15 +183,15 @@ impl PyExpr {
min_periods: usize,
closed: Wrap<ClosedWindow>,
ddof: u8,
) -> Self {
) -> PyResult<Self> {
let options = RollingOptionsDynamicWindow {
window_size: Duration::parse(window_size),
window_size: Duration::try_parse(window_size).map_err(PyPolarsErr::from)?,
min_periods,
closed_window: closed.0,
fn_params: Some(RollingFnParams::Var(RollingVarParams { ddof })),
};

self.inner.clone().rolling_std_by(by.inner, options).into()
Ok(self.inner.clone().rolling_std_by(by.inner, options).into())
}

#[pyo3(signature = (window_size, weights, min_periods, center, ddof))]
Expand Down Expand Up @@ -222,15 +223,15 @@ impl PyExpr {
min_periods: usize,
closed: Wrap<ClosedWindow>,
ddof: u8,
) -> Self {
) -> PyResult<Self> {
let options = RollingOptionsDynamicWindow {
window_size: Duration::parse(window_size),
window_size: Duration::try_parse(window_size).map_err(PyPolarsErr::from)?,
min_periods,
closed_window: closed.0,
fn_params: Some(RollingFnParams::Var(RollingVarParams { ddof })),
};

self.inner.clone().rolling_var_by(by.inner, options).into()
Ok(self.inner.clone().rolling_var_by(by.inner, options).into())
}

#[pyo3(signature = (window_size, weights, min_periods, center))]
Expand Down Expand Up @@ -259,17 +260,18 @@ impl PyExpr {
window_size: &str,
min_periods: usize,
closed: Wrap<ClosedWindow>,
) -> Self {
) -> PyResult<Self> {
let options = RollingOptionsDynamicWindow {
window_size: Duration::parse(window_size),
window_size: Duration::try_parse(window_size).map_err(PyPolarsErr::from)?,
min_periods,
closed_window: closed.0,
fn_params: None,
};
self.inner
Ok(self
.inner
.clone()
.rolling_median_by(by.inner, options)
.into()
.into())
}

#[pyo3(signature = (quantile, interpolation, window_size, weights, min_periods, center))]
Expand Down Expand Up @@ -306,18 +308,19 @@ impl PyExpr {
window_size: &str,
min_periods: usize,
closed: Wrap<ClosedWindow>,
) -> Self {
) -> PyResult<Self> {
let options = RollingOptionsDynamicWindow {
window_size: Duration::parse(window_size),
window_size: Duration::try_parse(window_size).map_err(PyPolarsErr::from)?,
min_periods,
closed_window: closed.0,
fn_params: None,
};

self.inner
Ok(self
.inner
.clone()
.rolling_quantile_by(by.inner, interpolation.0, quantile, options)
.into()
.into())
}

fn rolling_skew(&self, window_size: usize, bias: bool) -> Self {
Expand Down
46 changes: 28 additions & 18 deletions crates/polars-python/src/functions/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,12 @@ pub fn date_range(
end: PyExpr,
interval: &str,
closed: Wrap<ClosedWindow>,
) -> PyExpr {
) -> PyResult<PyExpr> {
let start = start.inner;
let end = end.inner;
let interval = Duration::parse(interval);
let interval = Duration::try_parse(interval).map_err(PyPolarsErr::from)?;
let closed = closed.0;
dsl::date_range(start, end, interval, closed).into()
Ok(dsl::date_range(start, end, interval, closed).into())
}

#[pyfunction]
Expand All @@ -85,12 +85,12 @@ pub fn date_ranges(
end: PyExpr,
interval: &str,
closed: Wrap<ClosedWindow>,
) -> PyExpr {
) -> PyResult<PyExpr> {
let start = start.inner;
let end = end.inner;
let interval = Duration::parse(interval);
let interval = Duration::try_parse(interval).map_err(PyPolarsErr::from)?;
let closed = closed.0;
dsl::date_ranges(start, end, interval, closed).into()
Ok(dsl::date_ranges(start, end, interval, closed).into())
}

#[pyfunction]
Expand All @@ -102,14 +102,14 @@ pub fn datetime_range(
closed: Wrap<ClosedWindow>,
time_unit: Option<Wrap<TimeUnit>>,
time_zone: Option<Wrap<TimeZone>>,
) -> PyExpr {
) -> PyResult<PyExpr> {
let start = start.inner;
let end = end.inner;
let every = Duration::parse(every);
let every = Duration::try_parse(every).map_err(PyPolarsErr::from)?;
let closed = closed.0;
let time_unit = time_unit.map(|x| x.0);
let time_zone = time_zone.map(|x| x.0);
dsl::datetime_range(start, end, every, closed, time_unit, time_zone).into()
Ok(dsl::datetime_range(start, end, every, closed, time_unit, time_zone).into())
}

#[pyfunction]
Expand All @@ -121,30 +121,40 @@ pub fn datetime_ranges(
closed: Wrap<ClosedWindow>,
time_unit: Option<Wrap<TimeUnit>>,
time_zone: Option<Wrap<TimeZone>>,
) -> PyExpr {
) -> PyResult<PyExpr> {
let start = start.inner;
let end = end.inner;
let every = Duration::parse(every);
let every = Duration::try_parse(every).map_err(PyPolarsErr::from)?;
let closed = closed.0;
let time_unit = time_unit.map(|x| x.0);
let time_zone = time_zone.map(|x| x.0);
dsl::datetime_ranges(start, end, every, closed, time_unit, time_zone).into()
Ok(dsl::datetime_ranges(start, end, every, closed, time_unit, time_zone).into())
}

#[pyfunction]
pub fn time_range(start: PyExpr, end: PyExpr, every: &str, closed: Wrap<ClosedWindow>) -> PyExpr {
pub fn time_range(
start: PyExpr,
end: PyExpr,
every: &str,
closed: Wrap<ClosedWindow>,
) -> PyResult<PyExpr> {
let start = start.inner;
let end = end.inner;
let every = Duration::parse(every);
let every = Duration::try_parse(every).map_err(PyPolarsErr::from)?;
let closed = closed.0;
dsl::time_range(start, end, every, closed).into()
Ok(dsl::time_range(start, end, every, closed).into())
}

#[pyfunction]
pub fn time_ranges(start: PyExpr, end: PyExpr, every: &str, closed: Wrap<ClosedWindow>) -> PyExpr {
pub fn time_ranges(
start: PyExpr,
end: PyExpr,
every: &str,
closed: Wrap<ClosedWindow>,
) -> PyResult<PyExpr> {
let start = start.inner;
let end = end.inner;
let every = Duration::parse(every);
let every = Duration::try_parse(every).map_err(PyPolarsErr::from)?;
let closed = closed.0;
dsl::time_ranges(start, end, every, closed).into()
Ok(dsl::time_ranges(start, end, every, closed).into())
}
18 changes: 9 additions & 9 deletions crates/polars-python/src/lazyframe/general.rs
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ impl PyLazyFrame {
offset: &str,
closed: Wrap<ClosedWindow>,
by: Vec<PyExpr>,
) -> PyLazyGroupBy {
) -> PyResult<PyLazyGroupBy> {
let closed_window = closed.0;
let ldf = self.ldf.clone();
let by = by
Expand All @@ -854,13 +854,13 @@ impl PyLazyFrame {
by,
RollingGroupOptions {
index_column: "".into(),
period: Duration::parse(period),
offset: Duration::parse(offset),
period: Duration::try_parse(period).map_err(PyPolarsErr::from)?,
offset: Duration::try_parse(offset).map_err(PyPolarsErr::from)?,
closed_window,
},
);

PyLazyGroupBy { lgb: Some(lazy_gb) }
Ok(PyLazyGroupBy { lgb: Some(lazy_gb) })
}

fn group_by_dynamic(
Expand All @@ -874,7 +874,7 @@ impl PyLazyFrame {
closed: Wrap<ClosedWindow>,
group_by: Vec<PyExpr>,
start_by: Wrap<StartBy>,
) -> PyLazyGroupBy {
) -> PyResult<PyLazyGroupBy> {
let closed_window = closed.0;
let group_by = group_by
.into_iter()
Expand All @@ -885,9 +885,9 @@ impl PyLazyFrame {
index_column.inner,
group_by,
DynamicGroupOptions {
every: Duration::parse(every),
period: Duration::parse(period),
offset: Duration::parse(offset),
every: Duration::try_parse(every).map_err(PyPolarsErr::from)?,
period: Duration::try_parse(period).map_err(PyPolarsErr::from)?,
offset: Duration::try_parse(offset).map_err(PyPolarsErr::from)?,
label: label.0,
include_boundaries,
closed_window,
Expand All @@ -896,7 +896,7 @@ impl PyLazyFrame {
},
);

PyLazyGroupBy { lgb: Some(lazy_gb) }
Ok(PyLazyGroupBy { lgb: Some(lazy_gb) })
}

fn with_context(&self, contexts: Vec<Self>) -> Self {
Expand Down
Loading

0 comments on commit 75aa4a3

Please sign in to comment.