Skip to content

Commit

Permalink
chore: Make get_list_builder infallible (#19217)
Browse files Browse the repository at this point in the history
  • Loading branch information
ritchie46 authored Oct 13, 2024
1 parent cca31b3 commit ff10b38
Show file tree
Hide file tree
Showing 14 changed files with 42 additions and 55 deletions.
32 changes: 16 additions & 16 deletions crates/polars-core/src/chunked_array/builder/list/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,17 @@ pub fn get_list_builder(
value_capacity: usize,
list_capacity: usize,
name: PlSmallStr,
) -> PolarsResult<Box<dyn ListBuilderTrait>> {
) -> Box<dyn ListBuilderTrait> {
match inner_type_logical {
#[cfg(feature = "dtype-categorical")]
DataType::Categorical(Some(rev_map), ordering) => {
return Ok(create_categorical_chunked_listbuilder(
return create_categorical_chunked_listbuilder(
name,
*ordering,
list_capacity,
value_capacity,
rev_map.clone(),
))
)
},
#[cfg(feature = "dtype-categorical")]
DataType::Enum(Some(rev_map), ordering) => {
Expand All @@ -108,7 +108,7 @@ pub fn get_list_builder(
value_capacity,
(**rev_map).clone(),
);
return Ok(Box::new(list_builder));
return Box::new(list_builder);
},
_ => {},
}
Expand All @@ -123,36 +123,36 @@ pub fn get_list_builder(
value_capacity,
list_capacity,
);
Ok(Box::new(builder))
Box::new(builder)
},
#[cfg(feature = "dtype-struct")]
DataType::Struct(_) => Ok(Box::new(AnonymousOwnedListBuilder::new(
DataType::Struct(_) => Box::new(AnonymousOwnedListBuilder::new(
name,
list_capacity,
Some(inner_type_logical.clone()),
))),
DataType::Null => Ok(Box::new(ListNullChunkedBuilder::new(name, list_capacity))),
DataType::List(_) => Ok(Box::new(AnonymousOwnedListBuilder::new(
)),
DataType::Null => Box::new(ListNullChunkedBuilder::new(name, list_capacity)),
DataType::List(_) => Box::new(AnonymousOwnedListBuilder::new(
name,
list_capacity,
Some(inner_type_logical.clone()),
))),
)),
#[cfg(feature = "dtype-array")]
DataType::Array(..) => Ok(Box::new(AnonymousOwnedListBuilder::new(
DataType::Array(..) => Box::new(AnonymousOwnedListBuilder::new(
name,
list_capacity,
Some(inner_type_logical.clone()),
))),
)),
#[cfg(feature = "dtype-decimal")]
DataType::Decimal(_, _) => Ok(Box::new(
DataType::Decimal(_, _) => Box::new(
ListPrimitiveChunkedBuilder::<Int128Type>::new_with_values_type(
name,
list_capacity,
value_capacity,
physical_type,
inner_type_logical.clone(),
),
)),
),
_ => {
macro_rules! get_primitive_builder {
($type:ty) => {{
Expand Down Expand Up @@ -186,13 +186,13 @@ pub fn get_list_builder(
Box::new(builder)
}};
}
Ok(match_dtype_to_logical_apply_macro!(
match_dtype_to_logical_apply_macro!(
physical_type,
get_primitive_builder,
get_string_builder,
get_binary_builder,
get_bool_builder
))
)
},
}
}
6 changes: 2 additions & 4 deletions crates/polars-core/src/chunked_array/from_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,7 @@ where
capacity * 5,
capacity,
PlSmallStr::EMPTY,
)
.unwrap();
);

builder.append_series(v.borrow()).unwrap();
for s in it {
Expand Down Expand Up @@ -205,8 +204,7 @@ impl FromIterator<Option<Series>> for ListChunked {
capacity * 5,
capacity,
PlSmallStr::EMPTY,
)
.unwrap();
);

for _ in 0..init_null_count {
builder.append_null();
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-core/src/chunked_array/from_iterator_par.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ fn materialize_list(
value_capacity: usize,
list_capacity: usize,
) -> ListChunked {
let mut builder = get_list_builder(&dtype, value_capacity, list_capacity, name).unwrap();
let mut builder = get_list_builder(&dtype, value_capacity, list_capacity, name);
for v in vectors {
for val in v {
builder.append_opt_series(val.as_ref()).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-core/src/chunked_array/list/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ mod test {

#[test]
fn test_iter_list() {
let mut builder = get_list_builder(&DataType::Int32, 10, 10, PlSmallStr::EMPTY).unwrap();
let mut builder = get_list_builder(&DataType::Int32, 10, 10, PlSmallStr::EMPTY);
builder
.append_series(&Series::new(PlSmallStr::EMPTY, &[1, 2, 3]))
.unwrap();
Expand Down
10 changes: 5 additions & 5 deletions crates/polars-core/src/chunked_array/ops/explode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ mod test {

#[test]
fn test_explode_list() -> PolarsResult<()> {
let mut builder = get_list_builder(&DataType::Int32, 5, 5, PlSmallStr::from_static("a"))?;
let mut builder = get_list_builder(&DataType::Int32, 5, 5, PlSmallStr::from_static("a"));

builder
.append_series(&Series::new(PlSmallStr::EMPTY, &[1, 2, 3, 3]))
Expand Down Expand Up @@ -300,7 +300,7 @@ mod test {
#[test]
fn test_explode_empty_list_slot() -> PolarsResult<()> {
// primitive
let mut builder = get_list_builder(&DataType::Int32, 5, 5, PlSmallStr::from_static("a"))?;
let mut builder = get_list_builder(&DataType::Int32, 5, 5, PlSmallStr::from_static("a"));
builder
.append_series(&Series::new(PlSmallStr::EMPTY, &[1i32, 2]))
.unwrap();
Expand All @@ -319,7 +319,7 @@ mod test {
);

// more primitive
let mut builder = get_list_builder(&DataType::Int32, 5, 5, PlSmallStr::from_static("a"))?;
let mut builder = get_list_builder(&DataType::Int32, 5, 5, PlSmallStr::from_static("a"));
builder
.append_series(&Series::new(PlSmallStr::EMPTY, &[1i32]))
.unwrap();
Expand All @@ -344,7 +344,7 @@ mod test {
);

// string
let mut builder = get_list_builder(&DataType::String, 5, 5, PlSmallStr::from_static("a"))?;
let mut builder = get_list_builder(&DataType::String, 5, 5, PlSmallStr::from_static("a"));
builder
.append_series(&Series::new(PlSmallStr::EMPTY, &["abc"]))
.unwrap();
Expand Down Expand Up @@ -390,7 +390,7 @@ mod test {
);

// boolean
let mut builder = get_list_builder(&DataType::Boolean, 5, 5, PlSmallStr::from_static("a"))?;
let mut builder = get_list_builder(&DataType::Boolean, 5, 5, PlSmallStr::from_static("a"));
builder
.append_series(&Series::new(PlSmallStr::EMPTY, &[true]))
.unwrap();
Expand Down
3 changes: 1 addition & 2 deletions crates/polars-core/src/chunked_array/ops/full.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ impl ChunkFullNull for BinaryOffsetChunked {

impl ChunkFull<&Series> for ListChunked {
fn full(name: PlSmallStr, value: &Series, length: usize) -> ListChunked {
let mut builder =
get_list_builder(value.dtype(), value.len() * length, length, name).unwrap();
let mut builder = get_list_builder(value.dtype(), value.len() * length, length, name);
for _ in 0..length {
builder.append_series(value).unwrap();
}
Expand Down
23 changes: 7 additions & 16 deletions crates/polars-core/src/named_from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use chrono::NaiveDateTime;
#[cfg(feature = "dtype-time")]
use chrono::NaiveTime;

use crate::chunked_array::builder::{get_list_builder, AnonymousListBuilder};
use crate::chunked_array::builder::get_list_builder;
use crate::prelude::*;

pub trait NamedFrom<T, Phantom: ?Sized> {
Expand Down Expand Up @@ -135,22 +135,13 @@ impl<T: AsRef<[Series]>> NamedFrom<T, ListType> for Series {

let dt = series_slice[0].dtype();

// inner type is also list so we need the anonymous builder
if let DataType::List(_) = dt {
let mut builder = AnonymousListBuilder::new(name, list_cap, Some(dt.clone()));
for s in series_slice {
builder.append_series(s).unwrap();
}
builder.finish().into_series()
} else {
let values_cap = series_slice.iter().fold(0, |acc, s| acc + s.len());
let values_cap = series_slice.iter().fold(0, |acc, s| acc + s.len());

let mut builder = get_list_builder(dt, values_cap, list_cap, name).unwrap();
for series in series_slice {
builder.append_series(series).unwrap();
}
builder.finish().into_series()
let mut builder = get_list_builder(dt, values_cap, list_cap, name);
for series in series_slice {
builder.append_series(series).unwrap();
}
builder.finish().into_series()
}
}

Expand All @@ -165,7 +156,7 @@ impl<T: AsRef<[Option<Series>]>> NamedFrom<T, [Option<Series>]> for Series {
None => &DataType::Null,
};

let mut builder = get_list_builder(dt, values_cap, series_slice.len(), name).unwrap();
let mut builder = get_list_builder(dt, values_cap, series_slice.len(), name);
for series in series_slice {
builder.append_opt_series(series.as_ref()).unwrap();
}
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-core/src/series/any_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ fn any_values_to_list(
};

let mut builder =
get_list_builder(&list_inner_type, capacity * 5, capacity, PlSmallStr::EMPTY)?;
get_list_builder(&list_inner_type, capacity * 5, capacity, PlSmallStr::EMPTY);

for av in avs {
match av {
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/ops/reshape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ impl Series {
);

let mut builder =
get_list_builder(s_ref.dtype(), s_ref.len(), rows as usize, s.name().clone())?;
get_list_builder(s_ref.dtype(), s_ref.len(), rows as usize, s.name().clone());

let mut offset = 0u64;
for _ in 0..rows {
Expand All @@ -285,7 +285,7 @@ mod test {
fn test_to_list() -> PolarsResult<()> {
let s = Series::new("a".into(), &[1, 2, 3]);

let mut builder = get_list_builder(s.dtype(), s.len(), 1, s.name().clone())?;
let mut builder = get_list_builder(s.dtype(), s.len(), 1, s.name().clone());
builder.append_series(&s).unwrap();
let expected = builder.finish();

Expand Down
2 changes: 1 addition & 1 deletion crates/polars-expr/src/expressions/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ impl ApplyExpr {
let len = iters[0].size_hint().0;

let ca = if len == 0 {
let mut builder = get_list_builder(&field.dtype, len * 5, len, field.name)?;
let mut builder = get_list_builder(&field.dtype, len * 5, len, field.name);
for _ in 0..len {
container.clear();
for iter in &mut iters {
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-expr/src/expressions/gather.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ impl GatherExpr {
idx.series().len(),
groups.len(),
ac.series().name().clone(),
)?;
);

let iter = ac.iter_groups(false).zip(idx.iter_groups(false));
for (s, idx) in iter {
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-lazy/src/tests/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1486,7 +1486,7 @@ fn test_singleton_broadcast() -> PolarsResult<()> {
#[test]
fn test_list_in_select_context() -> PolarsResult<()> {
let s = Column::new("a".into(), &[1, 2, 3]);
let mut builder = get_list_builder(s.dtype(), s.len(), 1, s.name().clone()).unwrap();
let mut builder = get_list_builder(s.dtype(), s.len(), 1, s.name().clone());
builder.append_series(s.as_materialized_series()).unwrap();
let expected = builder.finish().into_column();

Expand Down
4 changes: 2 additions & 2 deletions crates/polars-ops/src/chunked_array/list/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,7 @@ pub trait ListNameSpaceImpl: AsList {
ca.get_values_size() + vals_size_other + 1,
length,
ca.name().clone(),
)?;
);
ca.into_iter().for_each(|opt_s| {
let opt_s = opt_s.map(|mut s| {
for append in &to_append {
Expand Down Expand Up @@ -690,7 +690,7 @@ pub trait ListNameSpaceImpl: AsList {
ca.get_values_size() + vals_size_other + 1,
length,
ca.name().clone(),
)?;
);

for _ in 0..ca.len() {
let mut acc = match first_iter.next().unwrap() {
Expand Down
3 changes: 1 addition & 2 deletions crates/polars-python/src/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,7 @@ fn iterator_to_list(
name: PlSmallStr,
capacity: usize,
) -> PyResult<ListChunked> {
let mut builder =
get_list_builder(dt, capacity * 5, capacity, name).map_err(PyPolarsErr::from)?;
let mut builder = get_list_builder(dt, capacity * 5, capacity, name);
for _ in 0..init_null_count {
builder.append_null()
}
Expand Down

0 comments on commit ff10b38

Please sign in to comment.