Skip to content

Commit

Permalink
depr(python,rust!): Rename take to gather (#12528)
Browse files Browse the repository at this point in the history
  • Loading branch information
stinodego authored Nov 17, 2023
1 parent 655391c commit 14cdad0
Show file tree
Hide file tree
Showing 44 changed files with 211 additions and 117 deletions.
4 changes: 2 additions & 2 deletions crates/polars-core/src/chunked_array/ops/gather.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub fn check_bounds_nulls(idx: &PrimitiveArray<IdxSize>, len: IdxSize) -> Polars
in_bounds |= ((*x < len) as u32) << i;
}
let m = mask.get_u32(32 * block_idx);
polars_ensure!(m == m & in_bounds, ComputeError: "take indices are out of bounds");
polars_ensure!(m == m & in_bounds, ComputeError: "gather indices are out of bounds");
}
Ok(())
}
Expand All @@ -33,7 +33,7 @@ pub fn check_bounds_ca(indices: &IdxCa, len: IdxSize) -> PolarsResult<()> {
check_bounds_nulls(a, len).is_ok()
}
});
polars_ensure!(all_valid, ComputeError: "take indices are out of bounds");
polars_ensure!(all_valid, ComputeError: "gather indices are out of bounds");
Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion crates/polars-lazy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ date_offset = ["polars-plan/date_offset"]
trigonometry = ["polars-plan/trigonometry"]
sign = ["polars-plan/sign"]
timezones = ["polars-plan/timezones"]
list_take = ["polars-ops/list_take", "polars-plan/list_take"]
list_gather = ["polars-ops/list_gather", "polars-plan/list_gather"]
list_count = ["polars-ops/list_count", "polars-plan/list_count"]

true_div = ["polars-plan/true_div"]
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-lazy/src/physical_plan/expressions/take.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl PhysicalExpr for TakeExpr {
// A previous aggregation may have updated the groups.
let groups = ac.groups();

// Determine the take indices.
// Determine the gather indices.
let idx: IdxCa = match groups.as_ref() {
GroupsProxy::Idx(groups) => {
if groups.all().iter().zip(idx).any(|(g, idx)| match idx {
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-lazy/src/physical_plan/planner/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ pub(crate) fn create_physical_expr(
node_to_expr(expression, expr_arena),
)))
},
Take {
Gather {
expr,
idx,
returns_scalar,
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-lazy/src/tests/aggregations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ fn take_aggregations() -> PolarsResult<()> {
.agg([
// keep the head as it test slice correctness
col("book")
.take(
.gather(
col("count")
.arg_sort(SortOptions {
descending: true,
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-ops/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ cross_join = []
chunked_ids = ["polars-core/chunked_ids"]
asof_join = ["polars-core/asof_join"]
semi_anti_join = []
list_take = []
list_gather = []
list_sets = []
list_any_all = []
list_drop_nulls = []
Expand Down
18 changes: 9 additions & 9 deletions crates/polars-ops/src/chunked_array/list/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use std::fmt::Write;
use arrow::legacy::kernels::list::sublist_get;
use arrow::legacy::prelude::ValueSize;
use polars_core::chunked_array::builder::get_list_builder;
#[cfg(feature = "list_take")]
#[cfg(feature = "list_gather")]
use polars_core::export::num::ToPrimitive;
#[cfg(feature = "list_take")]
#[cfg(feature = "list_gather")]
use polars_core::export::num::{NumCast, Signed, Zero};
#[cfg(feature = "diff")]
use polars_core::series::ops::NullBehavior;
Expand Down Expand Up @@ -318,8 +318,8 @@ pub trait ListNameSpaceImpl: AsList {
.cast(&ca.inner_dtype())
}

#[cfg(feature = "list_take")]
fn lst_take(&self, idx: &Series, null_on_oob: bool) -> PolarsResult<Series> {
#[cfg(feature = "list_gather")]
fn lst_gather(&self, idx: &Series, null_on_oob: bool) -> PolarsResult<Series> {
let list_ca = self.as_list();

let index_typed_index = |idx: &Series| {
Expand Down Expand Up @@ -633,15 +633,15 @@ pub trait ListNameSpaceImpl: AsList {

impl ListNameSpaceImpl for ListChunked {}

#[cfg(feature = "list_take")]
#[cfg(feature = "list_gather")]
fn take_series(s: &Series, idx: Series, null_on_oob: bool) -> PolarsResult<Series> {
let len = s.len();
let idx = cast_index(idx, len, null_on_oob)?;
let idx = idx.idx().unwrap();
s.take(idx)
}

#[cfg(feature = "list_take")]
#[cfg(feature = "list_gather")]
fn cast_signed_index_ca<T: PolarsNumericType>(idx: &ChunkedArray<T>, len: usize) -> Series
where
T::Native: Copy + PartialOrd + PartialEq + NumCast + Signed + Zero,
Expand All @@ -652,7 +652,7 @@ where
.into_series()
}

#[cfg(feature = "list_take")]
#[cfg(feature = "list_gather")]
fn cast_unsigned_index_ca<T: PolarsNumericType>(idx: &ChunkedArray<T>, len: usize) -> Series
where
T::Native: Copy + PartialOrd + ToPrimitive,
Expand All @@ -672,7 +672,7 @@ where
.into_series()
}

#[cfg(feature = "list_take")]
#[cfg(feature = "list_gather")]
fn cast_index(idx: Series, len: usize, null_on_oob: bool) -> PolarsResult<Series> {
let idx_null_count = idx.null_count();
use DataType::*;
Expand Down Expand Up @@ -736,7 +736,7 @@ fn cast_index(idx: Series, len: usize, null_on_oob: bool) -> PolarsResult<Series
};
polars_ensure!(
out.null_count() == idx_null_count || null_on_oob,
ComputeError: "take indices are out of bounds"
ComputeError: "gather indices are out of bounds"
);
Ok(out)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-plan/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ dtype-categorical = ["polars-core/dtype-categorical"]
dtype-struct = ["polars-core/dtype-struct"]
object = ["polars-core/object"]
date_offset = ["polars-time", "chrono"]
list_take = ["polars-ops/list_take"]
list_gather = ["polars-ops/list_gather"]
list_count = ["polars-ops/list_count"]
trigonometry = []
sign = []
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-plan/src/dsl/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub enum Expr {
expr: Box<Expr>,
options: SortOptions,
},
Take {
Gather {
expr: Box<Expr>,
idx: Box<Expr>,
returns_scalar: bool,
Expand Down
22 changes: 11 additions & 11 deletions crates/polars-plan/src/dsl/function_expr/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ pub enum ListFunction {
Slice,
Shift,
Get,
#[cfg(feature = "list_take")]
Take(bool),
#[cfg(feature = "list_gather")]
Gather(bool),
#[cfg(feature = "list_count")]
CountMatches,
Sum,
Expand Down Expand Up @@ -66,8 +66,8 @@ impl ListFunction {
Slice => mapper.with_same_dtype(),
Shift => mapper.with_same_dtype(),
Get => mapper.map_to_list_and_array_inner_dtype(),
#[cfg(feature = "list_take")]
Take(_) => mapper.with_same_dtype(),
#[cfg(feature = "list_gather")]
Gather(_) => mapper.with_same_dtype(),
#[cfg(feature = "list_count")]
CountMatches => mapper.with_dtype(IDX_DTYPE),
Sum => mapper.nested_sum_type(),
Expand Down Expand Up @@ -125,8 +125,8 @@ impl Display for ListFunction {
Slice => "slice",
Shift => "shift",
Get => "get",
#[cfg(feature = "list_take")]
Take(_) => "take",
#[cfg(feature = "list_gather")]
Gather(_) => "gather",
#[cfg(feature = "list_count")]
CountMatches => "count",
Sum => "sum",
Expand Down Expand Up @@ -186,8 +186,8 @@ impl From<ListFunction> for SpecialEq<Arc<dyn SeriesUdf>> {
Slice => wrap!(slice),
Shift => map_as_slice!(shift),
Get => wrap!(get),
#[cfg(feature = "list_take")]
Take(null_ob_oob) => map_as_slice!(take, null_ob_oob),
#[cfg(feature = "list_gather")]
Gather(null_ob_oob) => map_as_slice!(gather, null_ob_oob),
#[cfg(feature = "list_count")]
CountMatches => map_as_slice!(count_matches),
Sum => map!(sum),
Expand Down Expand Up @@ -434,8 +434,8 @@ pub(super) fn get(s: &mut [Series]) -> PolarsResult<Option<Series>> {
}
}

#[cfg(feature = "list_take")]
pub(super) fn take(args: &[Series], null_on_oob: bool) -> PolarsResult<Series> {
#[cfg(feature = "list_gather")]
pub(super) fn gather(args: &[Series], null_on_oob: bool) -> PolarsResult<Series> {
let ca = &args[0];
let idx = &args[1];
let ca = ca.list()?;
Expand All @@ -447,7 +447,7 @@ pub(super) fn take(args: &[Series], null_on_oob: bool) -> PolarsResult<Series> {
// make sure we return a list
out.reshape(&[-1, 1])
} else {
ca.lst_take(idx, null_on_oob)
ca.lst_gather(idx, null_on_oob)
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/polars-plan/src/dsl/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,10 @@ impl ListNameSpace {
/// # Arguments
/// - `null_on_oob`: Return a null when an index is out of bounds.
/// This behavior is more expensive than defaulting to returning an `Error`.
#[cfg(feature = "list_take")]
#[cfg(feature = "list_gather")]
pub fn take(self, index: Expr, null_on_oob: bool) -> Expr {
self.0.map_many_private(
FunctionExpr::ListExpr(ListFunction::Take(null_on_oob)),
FunctionExpr::ListExpr(ListFunction::Gather(null_on_oob)),
&[index],
false,
false,
Expand Down
6 changes: 3 additions & 3 deletions crates/polars-plan/src/dsl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,8 @@ impl Expr {
}

/// Take the values by idx.
pub fn take<E: Into<Expr>>(self, idx: E) -> Self {
Expr::Take {
pub fn gather<E: Into<Expr>>(self, idx: E) -> Self {
Expr::Gather {
expr: Box::new(self),
idx: Box::new(idx.into()),
returns_scalar: false,
Expand All @@ -451,7 +451,7 @@ impl Expr {

/// Take the values by a single index.
pub fn get<E: Into<Expr>>(self, idx: E) -> Self {
Expr::Take {
Expr::Gather {
expr: Box::new(self),
idx: Box::new(idx.into()),
returns_scalar: true,
Expand Down
8 changes: 4 additions & 4 deletions crates/polars-plan/src/logical_plan/aexpr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub enum AExpr {
expr: Node,
options: SortOptions,
},
Take {
Gather {
expr: Node,
idx: Node,
returns_scalar: bool,
Expand Down Expand Up @@ -226,7 +226,7 @@ impl AExpr {
| Window { .. }
| Count
| Slice { .. }
| Take { .. }
| Gather { .. }
| Nth(_)
=> true,
Alias(_, _)
Expand Down Expand Up @@ -268,7 +268,7 @@ impl AExpr {
},
Cast { expr, .. } => container.push(*expr),
Sort { expr, .. } => container.push(*expr),
Take { expr, idx, .. } => {
Gather { expr, idx, .. } => {
container.push(*idx);
// latest, so that it is popped first
container.push(*expr);
Expand Down Expand Up @@ -347,7 +347,7 @@ impl AExpr {
*left = inputs[1];
return self;
},
Take { expr, idx, .. } => {
Gather { expr, idx, .. } => {
*idx = inputs[0];
*expr = inputs[1];
return self;
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-plan/src/logical_plan/aexpr/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl AExpr {
Ok(field)
},
Sort { expr, .. } => arena.get(*expr).to_field(schema, ctxt, arena),
Take { expr, .. } => arena.get(*expr).to_field(schema, ctxt, arena),
Gather { expr, .. } => arena.get(*expr).to_field(schema, ctxt, arena),
SortBy { expr, .. } => arena.get(*expr).to_field(schema, ctxt, arena),
Filter { input, .. } => arena.get(*input).to_field(schema, ctxt, arena),
Agg(agg) => {
Expand Down
8 changes: 4 additions & 4 deletions crates/polars-plan/src/logical_plan/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ pub fn to_aexpr(expr: Expr, arena: &mut Arena<AExpr>) -> Node {
data_type,
strict,
},
Expr::Take {
Expr::Gather {
expr,
idx,
returns_scalar,
} => AExpr::Take {
} => AExpr::Gather {
expr: to_aexpr(*expr, arena),
idx: to_aexpr(*idx, arena),
returns_scalar,
Expand Down Expand Up @@ -404,14 +404,14 @@ pub fn node_to_expr(node: Node, expr_arena: &Arena<AExpr>) -> Expr {
options,
}
},
AExpr::Take {
AExpr::Gather {
expr,
idx,
returns_scalar,
} => {
let expr = node_to_expr(expr, expr_arena);
let idx = node_to_expr(idx, expr_arena);
Expr::Take {
Expr::Gather {
expr: Box::new(expr),
idx: Box::new(idx),
returns_scalar,
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-plan/src/logical_plan/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ impl Debug for Expr {
Filter { input, by } => {
write!(f, "{input:?}.filter({by:?})")
},
Take {
Gather {
expr,
idx,
returns_scalar,
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-plan/src/logical_plan/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ macro_rules! push_expr {
},
Cast { expr, .. } => $push(expr),
Sort { expr, .. } => $push(expr),
Take { expr, idx, .. } => {
Gather { expr, idx, .. } => {
$push(idx);
$push(expr);
},
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-plan/src/logical_plan/tree_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ impl UpperExp for AExpr {
options.descending as u8, options.nulls_last as u8, options.multithreaded as u8
)
},
AExpr::Take { .. } => "take",
AExpr::Gather { .. } => "gather",
AExpr::SortBy { descending, .. } => {
write!(f, "sort_by:")?;
for i in descending {
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-plan/src/logical_plan/visitor/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ impl AexprNode {
},
) => strict_l == strict_r && dtl == dtr,
(Sort { options: l, .. }, Sort { options: r, .. }) => l == r,
(Take { .. }, Take { .. })
(Gather { .. }, Gather { .. })
| (Filter { .. }, Filter { .. })
| (Ternary { .. }, Ternary { .. })
| (Count, Count)
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-plan/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ pub(crate) fn get_single_leaf(expr: &Expr) -> PolarsResult<Arc<str>> {
for e in expr {
match e {
Expr::Filter { input, .. } => return get_single_leaf(input),
Expr::Take { expr, .. } => return get_single_leaf(expr),
Expr::Gather { expr, .. } => return get_single_leaf(expr),
Expr::SortBy { expr, .. } => return get_single_leaf(expr),
Expr::Window { function, .. } => return get_single_leaf(function),
Expr::Column(name) => return Ok(name.clone()),
Expand Down
2 changes: 1 addition & 1 deletion crates/polars/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ to_dummies = ["polars-ops/to_dummies"]
bigidx = ["polars-core/bigidx", "polars-lazy?/bigidx", "polars-ops/big_idx"]
list_to_struct = ["polars-ops/list_to_struct", "polars-lazy?/list_to_struct"]
list_count = ["polars-ops/list_count", "polars-lazy?/list_count"]
list_take = ["polars-ops/list_take", "polars-lazy?/list_take"]
list_gather = ["polars-ops/list_gather", "polars-lazy?/list_gather"]
describe = ["polars-core/describe"]
timezones = ["polars-core/timezones", "polars-lazy?/timezones", "polars-io/timezones"]
string_pad = ["polars-lazy?/string_pad", "polars-ops/string_pad"]
Expand Down
2 changes: 1 addition & 1 deletion crates/polars/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@
//! - `interpolate` [interpolate None values](polars_ops::chunked_array::interpolate)
//! - `extract_jsonpath` - [Run jsonpath queries on Utf8Chunked](https://goessner.net/articles/JsonPath/)
//! - `list` - List utils.
//! - `list_take` take sublist by multiple indices
//! - `list_gather` take sublist by multiple indices
//! - `rank` - Ranking algorithms.
//! - `moment` - kurtosis and skew statistics
//! - `ewma` - Exponential moving average windows
Expand Down
2 changes: 1 addition & 1 deletion crates/polars/tests/it/core/ops/take.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::*;

#[test]
fn test_list_take_nulls_and_empty() {
fn test_list_gather_nulls_and_empty() {
let a: &[i32] = &[];
let a = Series::new("", a);
let b = Series::new("", &[None, Some(a.clone())]);
Expand Down
Loading

0 comments on commit 14cdad0

Please sign in to comment.