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): Remove has_validity, use has_nulls #17519

Merged
merged 3 commits into from
Jul 16, 2024
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
8 changes: 0 additions & 8 deletions crates/polars-arrow/src/legacy/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,6 @@ pub trait ListFromIter {
}
impl ListFromIter for ListArray<i64> {}

pub trait PolarsArray: Array {
fn has_validity(&self) -> bool {
self.validity().is_some()
}
}

impl<A: Array + ?Sized> PolarsArray for A {}

fn is_nested_null(data_type: &ArrowDataType) -> bool {
match data_type {
ArrowDataType::Null => true,
Expand Down
1 change: 0 additions & 1 deletion crates/polars-core/src/chunked_array/builder/list/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ mod primitive;
pub use anonymous::*;
use arrow::legacy::array::list::AnonymousBuilder;
use arrow::legacy::array::null::MutableNullArray;
use arrow::legacy::prelude::*;
pub use binary::*;
pub use boolean::*;
#[cfg(feature = "dtype-categorical")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ where
let values = self.builder.mut_values();

ca.downcast_iter().for_each(|arr| {
if !arr.has_validity() {
if arr.null_count() == 0 {
values.extend_from_slice(arr.values().as_slice())
} else {
// SAFETY:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ impl LogicalType for CategoricalChunked {

let f = |idx: u32| mapping.get(idx);

if !self.physical.has_validity() {
if !self.physical.has_nulls() {
self.physical
.into_no_null_iter()
.for_each(|idx| builder.append_value(f(idx)));
Expand Down
7 changes: 3 additions & 4 deletions crates/polars-core/src/chunked_array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,9 @@ impl<T: PolarsDataType> ChunkedArray<T> {
}

#[inline]
/// Return if any the chunks in this [`ChunkedArray`] have a validity bitmap.
/// no bitmap means no null values.
pub fn has_validity(&self) -> bool {
self.iter_validities().any(|valid| valid.is_some())
/// Return if any the chunks in this [`ChunkedArray`] have nulls.
pub fn has_nulls(&self) -> bool {
self.null_count > 0
}

/// Shrink the capacity of this array to fit its length.
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-core/src/chunked_array/ops/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ impl<'a> ChunkApply<'a, Series> for ListChunked {
out
};
let mut ca: ListChunked = {
if !self.has_validity() {
if !self.has_nulls() {
self.into_no_null_iter()
.map(&mut function)
.collect_trusted()
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-core/src/chunked_array/ops/chunkops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ impl<T: PolarsObject> ObjectChunked<T> {

// todo! use iterators once implemented
// no_null path
if !self.has_validity() {
if !self.has_nulls() {
for arr in chunks {
for idx in 0..arr.len() {
builder.append_value(arr.value(idx).clone())
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-core/src/chunked_array/ops/explode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ where
// value and collect the indices.
// because the length of the array is not known, we first collect the null indexes, offset
// with the insertion of empty rows (as None) and later create a validity bitmap
if arr.has_validity() {
if arr.null_count() > 0 {
let validity_values = arr.validity().unwrap();

for &o in &offsets[1..] {
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-core/src/chunked_array/ops/nulls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::chunked_array::metadata::MetadataProperties;
impl<T: PolarsDataType> ChunkedArray<T> {
/// Get a mask of the null values.
pub fn is_null(&self) -> BooleanChunked {
if !self.has_validity() {
if !self.has_nulls() {
return BooleanChunked::full(self.name(), false, self.len());
}
// dispatch to non-generic function
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/chunked_array/ops/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ where
idx: I,
value: Option<T::Native>,
) -> PolarsResult<Self> {
if !self.has_validity() {
if !self.has_nulls() {
if let Some(value) = value {
// Fast path uses kernel.
if self.chunks.len() == 1 {
Expand Down Expand Up @@ -94,7 +94,7 @@ where
check_bounds!(self, mask);

// Fast path uses the kernel in polars-arrow.
if let (Some(value), false) = (value, mask.has_validity()) {
if let (Some(value), false) = (value, mask.has_nulls()) {
let (left, mask) = align_chunks_binary(self, mask);

// Apply binary kernel.
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-core/src/chunked_array/ops/unique/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ where

macro_rules! arg_unique_ca {
($ca:expr) => {{
match $ca.has_validity() {
match $ca.has_nulls() {
false => arg_unique($ca.into_no_null_iter(), $ca.len()),
_ => arg_unique($ca.iter(), $ca.len()),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ where
} else if idx.len() == 1 {
self.get(first as usize).map(|sum| sum.to_f64().unwrap())
} else {
match (self.has_validity(), self.chunks.len()) {
match (self.has_nulls(), self.chunks.len()) {
(false, 1) => {
take_agg_no_null_primitive_iter_unchecked::<_, f64, _, _>(
arr,
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-core/src/frame/group_by/into_groups.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ where
.collect::<Vec<_>>();
group_by_threaded_iter(&keys, n_partitions, sorted)
}
} else if !ca.has_validity() {
} else if !ca.has_nulls() {
group_by(ca.into_no_null_iter(), sorted)
} else {
group_by(ca.iter(), sorted)
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-core/src/frame/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@ impl DataFrame {
};

// fast path for no nulls in df
if iter.clone().all(|s| !s.has_validity()) {
if iter.clone().all(|s| !s.has_nulls()) {
return Ok(self.clone());
}

Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ impl SeriesTrait for SeriesWrap<ArrayChunked> {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

fn is_null(&self) -> BooleanChunked {
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ impl SeriesTrait for SeriesWrap<BinaryChunked> {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

#[cfg(feature = "algorithm_group_by")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ impl SeriesTrait for SeriesWrap<BinaryOffsetChunked> {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

fn is_null(&self) -> BooleanChunked {
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/boolean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ impl SeriesTrait for SeriesWrap<BooleanChunked> {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

#[cfg(feature = "algorithm_group_by")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ impl SeriesTrait for SeriesWrap<CategoricalChunked> {
self.0.physical().null_count()
}

fn has_validity(&self) -> bool {
self.0.physical().has_validity()
fn has_nulls(&self) -> bool {
self.0.physical().has_nulls()
}

#[cfg(feature = "algorithm_group_by")]
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/date.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,8 @@ impl SeriesTrait for SeriesWrap<DateChunked> {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

#[cfg(feature = "algorithm_group_by")]
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,8 @@ impl SeriesTrait for SeriesWrap<DatetimeChunked> {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

#[cfg(feature = "algorithm_group_by")]
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,8 @@ impl SeriesTrait for SeriesWrap<DecimalChunked> {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

fn is_null(&self) -> BooleanChunked {
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/duration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,8 @@ impl SeriesTrait for SeriesWrap<DurationChunked> {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

#[cfg(feature = "algorithm_group_by")]
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/floats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,8 @@ macro_rules! impl_dyn_series {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

#[cfg(feature = "algorithm_group_by")]
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ impl SeriesTrait for SeriesWrap<ListChunked> {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

#[cfg(feature = "algorithm_group_by")]
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,8 @@ macro_rules! impl_dyn_series {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

#[cfg(feature = "algorithm_group_by")]
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ impl SeriesTrait for NullChunked {
self.length as usize
}

fn has_validity(&self) -> bool {
true
fn has_nulls(&self) -> bool {
self.len() > 0
}

fn rechunk(&self) -> Series {
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ where
ObjectChunked::null_count(&self.0)
}

fn has_validity(&self) -> bool {
ObjectChunked::has_validity(&self.0)
fn has_nulls(&self) -> bool {
ObjectChunked::has_nulls(&self.0)
}

fn unique(&self) -> PolarsResult<Series> {
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ impl SeriesTrait for SeriesWrap<StringChunked> {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

#[cfg(feature = "algorithm_group_by")]
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/struct__.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ impl SeriesTrait for SeriesWrap<StructChunked> {
Ok(IdxCa::from_vec(self.name(), first))
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

fn is_null(&self) -> BooleanChunked {
Expand Down
4 changes: 2 additions & 2 deletions crates/polars-core/src/series/implementations/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ impl SeriesTrait for SeriesWrap<TimeChunked> {
self.0.null_count()
}

fn has_validity(&self) -> bool {
self.0.has_validity()
fn has_nulls(&self) -> bool {
self.0.has_nulls()
}

#[cfg(feature = "algorithm_group_by")]
Expand Down
5 changes: 2 additions & 3 deletions crates/polars-core/src/series/series_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,9 +364,8 @@ pub trait SeriesTrait:
/// Count the null values.
fn null_count(&self) -> usize;

/// Return if any the chunks in this `[ChunkedArray]` have a validity bitmap.
/// no bitmap means no null values.
fn has_validity(&self) -> bool;
/// Return if any the chunks in this `[ChunkedArray]` have nulls.
fn has_nulls(&self) -> bool;

/// Get unique values in the Series.
fn unique(&self) -> PolarsResult<Series> {
Expand Down
16 changes: 5 additions & 11 deletions crates/polars-ops/src/chunked_array/list/min_max.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use arrow::array::{Array, PrimitiveArray};
use arrow::bitmap::Bitmap;
use arrow::compute::utils::combine_validities_and;
use arrow::types::NativeType;
use polars_compute::min_max::MinMaxKernel;
use polars_core::prelude::*;
Expand Down Expand Up @@ -36,16 +37,9 @@ where
{
let values = arr.as_any().downcast_ref::<PrimitiveArray<T>>().unwrap();
let values = values.values().as_slice();
let mut out = min_between_offsets(values, offsets);

if let Some(validity) = validity {
if out.has_validity() {
out.apply_validity(|other_validity| validity & &other_validity)
} else {
out = out.with_validity(Some(validity.clone()));
}
}
Box::new(out)
let out = min_between_offsets(values, offsets);
let new_validity = combine_validities_and(out.validity(), validity);
out.with_validity(new_validity).to_boxed()
}

fn min_list_numerical(ca: &ListChunked, inner_type: &DataType) -> Series {
Expand Down Expand Up @@ -148,7 +142,7 @@ where
let mut out = max_between_offsets(values, offsets);

if let Some(validity) = validity {
if out.has_validity() {
if out.null_count() > 0 {
out.apply_validity(|other_validity| validity & &other_validity)
} else {
out = out.with_validity(Some(validity.clone()));
Expand Down
Loading
Loading