Skip to content

Commit

Permalink
refactor(rust, python): Rename kwarg reverse to descending (pola-rs#6914
Browse files Browse the repository at this point in the history
)
  • Loading branch information
zundertj authored and josemasar committed Feb 21, 2023
1 parent 6fcca9d commit 0860b98
Show file tree
Hide file tree
Showing 71 changed files with 553 additions and 478 deletions.
10 changes: 5 additions & 5 deletions polars/polars-arrow/src/kernels/sort_partition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ use arrow::types::NativeType;
use crate::index::IdxSize;

/// Find partition indexes such that every partition contains unique groups.
fn find_partition_points<T>(values: &[T], n: usize, reverse: bool) -> Vec<usize>
fn find_partition_points<T>(values: &[T], n: usize, descending: bool) -> Vec<usize>
where
T: Debug + NativeType + PartialOrd,
{
let len = values.len();
if n > len {
return find_partition_points(values, len / 2, reverse);
return find_partition_points(values, len / 2, descending);
}
if n < 2 {
return vec![];
Expand All @@ -31,7 +31,7 @@ where
let part = &values[start_idx..end_idx];

let latest_val = values[end_idx];
let idx = if reverse {
let idx = if descending {
part.partition_point(|v| *v > latest_val)
} else {
part.partition_point(|v| *v < latest_val)
Expand All @@ -46,11 +46,11 @@ where
partition_points
}

pub fn create_clean_partitions<T>(values: &[T], n: usize, reverse: bool) -> Vec<&[T]>
pub fn create_clean_partitions<T>(values: &[T], n: usize, descending: bool) -> Vec<&[T]>
where
T: Debug + NativeType + PartialOrd,
{
let part_idx = find_partition_points(values, n, reverse);
let part_idx = find_partition_points(values, n, descending);
let mut out = Vec::with_capacity(n + 1);

let mut start_idx = 0_usize;
Expand Down
18 changes: 9 additions & 9 deletions polars/polars-core/src/chunked_array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,11 @@ bitflags! {
}}

impl<T: PolarsDataType> ChunkedArray<T> {
pub(crate) fn is_sorted_flag(&self) -> bool {
pub(crate) fn is_sorted_ascending_flag(&self) -> bool {
self.bit_settings.contains(Settings::SORTED_ASC)
}

pub(crate) fn is_sorted_reverse_flag(&self) -> bool {
pub(crate) fn is_sorted_descending_flag(&self) -> bool {
self.bit_settings.contains(Settings::SORTED_DSC)
}

Expand All @@ -171,9 +171,9 @@ impl<T: PolarsDataType> ChunkedArray<T> {
}

pub fn is_sorted_flag2(&self) -> IsSorted {
if self.is_sorted_flag() {
if self.is_sorted_ascending_flag() {
IsSorted::Ascending
} else if self.is_sorted_reverse_flag() {
} else if self.is_sorted_descending_flag() {
IsSorted::Descending
} else {
IsSorted::Not
Expand All @@ -188,15 +188,15 @@ impl<T: PolarsDataType> ChunkedArray<T> {
.remove(Settings::SORTED_ASC | Settings::SORTED_DSC);
}
IsSorted::Ascending => {
// // unset reverse sorted
// // unset descending sorted
self.bit_settings.remove(Settings::SORTED_DSC);
// set sorted
// set ascending sorted
self.bit_settings.insert(Settings::SORTED_ASC)
}
IsSorted::Descending => {
// unset sorted
// unset ascending sorted
self.bit_settings.remove(Settings::SORTED_ASC);
// set reverse sorted
// set descending sorted
self.bit_settings.insert(Settings::SORTED_DSC)
}
}
Expand Down Expand Up @@ -648,7 +648,7 @@ pub(crate) mod test {
let a = a.sort(false);
let b = a.into_iter().collect::<Vec<_>>();
assert_eq!(b, [Some("a"), Some("b"), Some("c")]);
assert_eq!(a.is_sorted_flag(), true);
assert_eq!(a.is_sorted_ascending_flag(), true);
}

#[test]
Expand Down
12 changes: 6 additions & 6 deletions polars/polars-core/src/chunked_array/ops/aggregate/quantile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ where
interpol: QuantileInterpolOptions,
) -> PolarsResult<Option<f64>> {
// in case of sorted data, the sort is free, so don't take quickselect route
if let (Ok(slice), false) = (self.cont_slice(), self.is_sorted_flag()) {
if let (Ok(slice), false) = (self.cont_slice(), self.is_sorted_ascending_flag()) {
let mut owned = slice.to_vec();
quantile_slice(&mut owned, quantile, interpol)
} else {
Expand All @@ -217,7 +217,7 @@ where
interpol: QuantileInterpolOptions,
) -> PolarsResult<Option<f64>> {
// in case of sorted data, the sort is free, so don't take quickselect route
let is_sorted = self.is_sorted_flag();
let is_sorted = self.is_sorted_ascending_flag();
if let (Some(slice), false) = (self.cont_slice_mut(), is_sorted) {
quantile_slice(slice, quantile, interpol)
} else {
Expand All @@ -238,7 +238,7 @@ impl ChunkQuantile<f32> for Float32Chunked {
interpol: QuantileInterpolOptions,
) -> PolarsResult<Option<f32>> {
// in case of sorted data, the sort is free, so don't take quickselect route
let out = if let (Ok(slice), false) = (self.cont_slice(), self.is_sorted_flag()) {
let out = if let (Ok(slice), false) = (self.cont_slice(), self.is_sorted_ascending_flag()) {
let mut owned = slice.to_vec();
let owned = f32_to_ordablef32(&mut owned);
quantile_slice(owned, quantile, interpol)
Expand All @@ -260,7 +260,7 @@ impl ChunkQuantile<f64> for Float64Chunked {
interpol: QuantileInterpolOptions,
) -> PolarsResult<Option<f64>> {
// in case of sorted data, the sort is free, so don't take quickselect route
if let (Ok(slice), false) = (self.cont_slice(), self.is_sorted_flag()) {
if let (Ok(slice), false) = (self.cont_slice(), self.is_sorted_ascending_flag()) {
let mut owned = slice.to_vec();
let owned = f64_to_ordablef64(&mut owned);
quantile_slice(owned, quantile, interpol)
Expand All @@ -281,7 +281,7 @@ impl Float64Chunked {
interpol: QuantileInterpolOptions,
) -> PolarsResult<Option<f64>> {
// in case of sorted data, the sort is free, so don't take quickselect route
let is_sorted = self.is_sorted_flag();
let is_sorted = self.is_sorted_ascending_flag();
if let (Some(slice), false) = (self.cont_slice_mut(), is_sorted) {
let slice = f64_to_ordablef64(slice);
quantile_slice(slice, quantile, interpol)
Expand All @@ -303,7 +303,7 @@ impl Float32Chunked {
interpol: QuantileInterpolOptions,
) -> PolarsResult<Option<f32>> {
// in case of sorted data, the sort is free, so don't take quickselect route
let is_sorted = self.is_sorted_flag();
let is_sorted = self.is_sorted_ascending_flag();
if let (Some(slice), false) = (self.cont_slice_mut(), is_sorted) {
let slice = f32_to_ordablef32(slice);
quantile_slice(slice, quantile, interpol).map(|v| v.map(|v| v as f32))
Expand Down
4 changes: 2 additions & 2 deletions polars/polars-core/src/chunked_array/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,13 +493,13 @@ pub trait ChunkSort<T: PolarsDataType> {
fn sort_with(&self, options: SortOptions) -> ChunkedArray<T>;

/// Returned a sorted `ChunkedArray`.
fn sort(&self, reverse: bool) -> ChunkedArray<T>;
fn sort(&self, descending: bool) -> ChunkedArray<T>;

/// Retrieve the indexes needed to sort this array.
fn arg_sort(&self, options: SortOptions) -> IdxCa;

/// Retrieve the indexes need to sort this and the other arrays.
fn arg_sort_multiple(&self, _other: &[Series], _reverse: &[bool]) -> PolarsResult<IdxCa> {
fn arg_sort_multiple(&self, _other: &[Series], _descending: &[bool]) -> PolarsResult<IdxCa> {
Err(PolarsError::InvalidOperation(
"arg_sort_multiple not implemented for this dtype".into(),
))
Expand Down
20 changes: 10 additions & 10 deletions polars/polars-core/src/chunked_array/ops/sort/arg_sort.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use super::*;

#[inline]
fn default_order<T: PartialOrd + IsFloat>(a: &(IdxSize, T), b: &(IdxSize, T)) -> Ordering {
fn ascending_order<T: PartialOrd + IsFloat>(a: &(IdxSize, T), b: &(IdxSize, T)) -> Ordering {
compare_fn_nan_max(&a.1, &b.1)
}

#[inline]
fn reverse_order<T: PartialOrd + IsFloat>(a: &(IdxSize, T), b: &(IdxSize, T)) -> Ordering {
fn descending_order<T: PartialOrd + IsFloat>(a: &(IdxSize, T), b: &(IdxSize, T)) -> Ordering {
compare_fn_nan_max(&b.1, &a.1)
}

Expand All @@ -22,14 +22,14 @@ where
J: IntoIterator<Item = Option<T>>,
T: PartialOrd + Send + Sync + IsFloat,
{
let reverse = options.descending;
let descending = options.descending;
let nulls_last = options.nulls_last;

let mut vals = Vec::with_capacity(len - null_count);

// if we sort reverse, the nulls are last
// and need to be extended to the indices in reverse order
let null_cap = if reverse || nulls_last {
// if we sort descending, the nulls are last
// and need to be extended to the indices in descending order
let null_cap = if descending || nulls_last {
null_count
// if we sort normally, the nulls are first
// and can be extended with the sorted indices
Expand Down Expand Up @@ -58,14 +58,14 @@ where

arg_sort_branch(
vals.as_mut_slice(),
reverse,
default_order,
reverse_order,
descending,
ascending_order,
descending_order,
options.multithreaded,
);

let iter = vals.into_iter().map(|(idx, _v)| idx);
let idx = if reverse || nulls_last {
let idx = if descending || nulls_last {
let mut idx = Vec::with_capacity(len);
idx.extend(iter);
idx.extend(nulls_idx.into_iter().rev());
Expand Down
23 changes: 14 additions & 9 deletions polars/polars-core/src/chunked_array/ops/sort/arg_sort_multiple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,47 +5,52 @@ use super::*;
pub(crate) fn args_validate<T: PolarsDataType>(
ca: &ChunkedArray<T>,
other: &[Series],
reverse: &[bool],
descending: &[bool],
) -> PolarsResult<()> {
for s in other {
assert_eq!(ca.len(), s.len());
}
if other.len() != (reverse.len() - 1) {
if other.len() != (descending.len() - 1) {
return Err(PolarsError::ComputeError(
format!(
"The amount of ordering booleans: {} does not match that no. of Series: {}",
reverse.len(),
descending.len(),
other.len() + 1
)
.into(),
));
}

assert_eq!(other.len(), reverse.len() - 1);
assert_eq!(other.len(), descending.len() - 1);
Ok(())
}

pub(crate) fn arg_sort_multiple_impl<T: PartialOrd + Send + IsFloat + Copy>(
mut vals: Vec<(IdxSize, T)>,
other: &[Series],
reverse: &[bool],
descending: &[bool],
) -> PolarsResult<IdxCa> {
assert_eq!(reverse.len() - 1, other.len());
assert_eq!(descending.len() - 1, other.len());
let compare_inner: Vec<_> = other
.iter()
.map(|s| s.into_partial_ord_inner())
.collect_trusted();

let first_reverse = reverse[0];
let first_descending = descending[0];
vals.par_sort_by(|tpl_a, tpl_b| {
match (first_reverse, compare_fn_nan_max(&tpl_a.1, &tpl_b.1)) {
match (first_descending, compare_fn_nan_max(&tpl_a.1, &tpl_b.1)) {
// if ordering is equal, we check the other arrays until we find a non-equal ordering
// if we have exhausted all arrays, we keep the equal ordering.
(_, Ordering::Equal) => {
let idx_a = tpl_a.0 as usize;
let idx_b = tpl_b.0 as usize;
unsafe {
ordering_other_columns(&compare_inner, reverse.get_unchecked(1..), idx_a, idx_b)
ordering_other_columns(
&compare_inner,
descending.get_unchecked(1..),
idx_a,
idx_b,
)
}
}
(true, Ordering::Less) => Ordering::Greater,
Expand Down
20 changes: 10 additions & 10 deletions polars/polars-core/src/chunked_array/ops/sort/categorical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ fn sort_with_nulls<T: PartialOrd>(a: &Option<T>, b: &Option<T>) -> Ordering {
}

/// Default sorting nulls
pub fn order_default_null<T: PartialOrd>(a: &Option<T>, b: &Option<T>) -> Ordering {
pub fn order_ascending_null<T: PartialOrd>(a: &Option<T>, b: &Option<T>) -> Ordering {
sort_with_nulls(a, b)
}

/// Default sorting nulls
pub fn order_reverse_null<T: PartialOrd>(a: &Option<T>, b: &Option<T>) -> Ordering {
pub fn order_descending_null<T: PartialOrd>(a: &Option<T>, b: &Option<T>) -> Ordering {
sort_with_nulls(b, a)
}

Expand Down Expand Up @@ -60,8 +60,8 @@ impl CategoricalChunked {
arg_sort_branch(
vals.as_mut_slice(),
options.descending,
|(_, a), (_, b)| order_default_null(a, b),
|(_, a), (_, b)| order_reverse_null(a, b),
|(_, a), (_, b)| order_ascending_null(a, b),
|(_, a), (_, b)| order_descending_null(a, b),
options.multithreaded,
);
let cats: NoNull<UInt32Chunked> =
Expand Down Expand Up @@ -94,10 +94,10 @@ impl CategoricalChunked {

/// Returned a sorted `ChunkedArray`.
#[must_use]
pub fn sort(&self, reverse: bool) -> CategoricalChunked {
pub fn sort(&self, descending: bool) -> CategoricalChunked {
self.sort_with(SortOptions {
nulls_last: false,
descending: reverse,
descending,
multithreaded: true,
})
}
Expand All @@ -123,10 +123,10 @@ impl CategoricalChunked {
pub(crate) fn arg_sort_multiple(
&self,
other: &[Series],
reverse: &[bool],
descending: &[bool],
) -> PolarsResult<IdxCa> {
if self.use_lexical_sort() {
args_validate(self.logical(), other, reverse)?;
args_validate(self.logical(), other, descending)?;
let mut count: IdxSize = 0;
let vals: Vec<_> = self
.iter_str()
Expand All @@ -137,9 +137,9 @@ impl CategoricalChunked {
})
.collect_trusted();

arg_sort_multiple_impl(vals, other, reverse)
arg_sort_multiple_impl(vals, other, descending)
} else {
self.logical().arg_sort_multiple(other, reverse)
self.logical().arg_sort_multiple(other, descending)
}
}
}
Expand Down

0 comments on commit 0860b98

Please sign in to comment.