Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Make Duration parsing fallible and not panic
Browse files Browse the repository at this point in the history
coastalwhite committed Oct 28, 2024
1 parent 60d0721 commit 73222a6
Showing 6 changed files with 103 additions and 79 deletions.
6 changes: 3 additions & 3 deletions crates/polars-python/src/dataframe/general.rs
Original file line number Diff line number Diff line change
@@ -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())
18 changes: 10 additions & 8 deletions crates/polars-python/src/expr/general.rs
Original file line number Diff line number Diff line change
@@ -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;

@@ -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 {
@@ -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(
55 changes: 29 additions & 26 deletions crates/polars-python/src/expr/rolling.rs
Original file line number Diff line number Diff line change
@@ -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};

@@ -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))]
@@ -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))]
@@ -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))]
@@ -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))]
@@ -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))]
@@ -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))]
@@ -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))]
@@ -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 {
46 changes: 28 additions & 18 deletions crates/polars-python/src/functions/range.rs
Original file line number Diff line number Diff line change
@@ -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]
@@ -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]
@@ -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]
@@ -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
@@ -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
@@ -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(
@@ -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()
@@ -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,
@@ -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 {
39 changes: 24 additions & 15 deletions crates/polars-time/src/windows/duration.rs
Original file line number Diff line number Diff line change
@@ -153,31 +153,39 @@ impl Duration {
/// # Panics
/// If the given str is invalid for any reason.
pub fn parse(duration: &str) -> Self {
Self::_parse(duration, false)
Self::try_parse(duration).unwrap()
}

#[doc(hidden)]
/// Parse SQL-style "interval" string to Duration. Handles verbose
/// units (such as 'year', 'minutes', etc.) and whitespace, as
/// well as being case-insensitive.
pub fn parse_interval(interval: &str) -> Self {
Self::try_parse_interval(interval).unwrap()
}

pub fn try_parse(duration: &str) -> PolarsResult<Self> {
Self::_parse(duration, false)
}

pub fn try_parse_interval(interval: &str) -> PolarsResult<Self> {
Self::_parse(&interval.to_ascii_lowercase(), true)
}

fn _parse(s: &str, as_interval: bool) -> Self {
fn _parse(s: &str, as_interval: bool) -> PolarsResult<Self> {
let s = if as_interval { s.trim_start() } else { s };

let parse_type = if as_interval { "interval" } else { "duration" };
let num_minus_signs = s.matches('-').count();
if num_minus_signs > 1 {
panic!("{} string can only have a single minus sign", parse_type)
polars_bail!(InvalidOperation: "{} string can only have a single minus sign", parse_type);
}
if num_minus_signs > 0 {
if as_interval {
// TODO: intervals need to support per-element minus signs
panic!("minus signs are not currently supported in interval strings")
polars_bail!(InvalidOperation: "minus signs are not currently supported in interval strings");
} else if !s.starts_with('-') {
panic!("only a single minus sign is allowed, at the front of the string")
polars_bail!(InvalidOperation: "only a single minus sign is allowed, at the front of the string");
}
}
let mut months = 0;
@@ -211,12 +219,12 @@ impl Duration {

while let Some((i, mut ch)) = iter.next() {
if !ch.is_ascii_digit() {
let n = s[start..i].parse::<i64>().unwrap_or_else(|_| {
panic!(
let Ok(n) = s[start..i].parse::<i64>() else {
polars_bail!(InvalidOperation:
"expected leading integer in the {} string, found {}",
parse_type, ch
)
});
);
};

loop {
match ch {
@@ -233,10 +241,10 @@ impl Duration {
}
}
if unit.is_empty() {
panic!(
polars_bail!(InvalidOperation:
"expected a unit to follow integer in the {} string '{}'",
parse_type, s
)
);
}
match &*unit {
// matches that are allowed for both duration/interval
@@ -270,24 +278,25 @@ impl Duration {
"year" | "years" => months += n * 12,
_ => {
let valid_units = "'year', 'month', 'quarter', 'week', 'day', 'hour', 'minute', 'second', 'millisecond', 'microsecond', 'nanosecond'";
panic!("unit: '{unit}' not supported; available units include: {} (and their plurals)", valid_units)
polars_bail!(InvalidOperation: "unit: '{unit}' not supported; available units include: {} (and their plurals)", valid_units);
},
},
_ => {
panic!("unit: '{unit}' not supported; available units are: 'y', 'mo', 'q', 'w', 'd', 'h', 'm', 's', 'ms', 'us', 'ns'")
polars_bail!(InvalidOperation: "unit: '{unit}' not supported; available units are: 'y', 'mo', 'q', 'w', 'd', 'h', 'm', 's', 'ms', 'us', 'ns'");
},
}
unit.clear();
}
}
Duration {

Ok(Duration {
nsecs: nsecs.abs(),
days: days.abs(),
weeks: weeks.abs(),
months: months.abs(),
negative,
parsed_int,
}
})
}

fn to_positive(v: i64) -> (bool, i64) {

0 comments on commit 73222a6

Please sign in to comment.