Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(rust, python): Rename kwarg reverse to descending #6914

Merged
merged 1 commit into from
Feb 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading