clean up

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>

Author: connortsui20

vortex-dtype/src/nullability.rs

@@ -15,72 +15,21 @@ pub enum Nullability {
 }
 
 impl Nullability {
-    /// Returns `Some(f())` if the the nullability is [`Nullable`](Self::Nullable), otherwise
-    /// returns `None`.
+    /// Returns `true` if the nullability is [`Nullable`](Self::Nullable), otherwise returns
+    /// `false`.
     ///
     /// # Examples
     ///
     /// ```
     /// use vortex_dtype::Nullability::*;
     ///
-    /// assert_eq!(NonNullable.is_nullable_then(|| 0), None);
-    /// assert_eq!(Nullable.is_nullable_then(|| 0), Some(0));
+    /// assert!(!NonNullable.is_nullable());
+    /// assert!(Nullable.is_nullable());
     /// ```
-    ///
-    /// ```
-    /// # use vortex_dtype::Nullability::*;
-    /// #
-    /// let mut a = 0;
-    ///
-    /// Nullable.is_nullable_then(|| { a += 1; });
-    /// NonNullable.is_nullable_then(|| { a += 1; });
-    ///
-    /// // `a` is incremented once because the closure is evaluated lazily by `is_nullable_then`.
-    /// assert_eq!(a, 1);
-    /// ```
-    ///
-    /// Inspired by the [`bool::then`] function.
-    pub fn is_nullable_then<T, F: FnOnce() -> T>(self, f: F) -> Option<T> {
-        match self {
-            Nullability::NonNullable => None,
-            Nullability::Nullable => Some(f()),
-        }
-    }
-
-    /// Returns `Some(t)` if the the nullability is [`Nullable`](Self::Nullable), otherwise returns
-    /// `None`.
-    ///
-    /// Arguments passed to `is_nullable_then_some` are eagerly evaluated; if you are passing the
-    /// result of a function call, it is recommended to use [`then`](bool::then), which is lazily
-    /// evaluated.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// use vortex_dtype::Nullability::*;
-    ///
-    /// assert_eq!(NonNullable.is_nullable_then_some(0), None);
-    /// assert_eq!(Nullable.is_nullable_then_some(0), Some(0));
-    /// ```
-    ///
-    /// ```
-    /// # use vortex_dtype::Nullability::*;
-    /// #
-    /// let mut a = 0;
-    /// let mut function_with_side_effects = || { a += 1; };
-    ///
-    /// Nullable.is_nullable_then_some(function_with_side_effects());
-    /// NonNullable.is_nullable_then_some(function_with_side_effects());
-    ///
-    /// // `a` is incremented twice because the value passed to `then_some` is evaluated eagerly.
-    /// assert_eq!(a, 2);
-    /// ```
-    ///
-    /// Inspired by the [`bool::then_some`] function.
-    pub fn is_nullable_then_some<T>(self, t: T) -> Option<T> {
+    pub fn is_nullable(&self) -> bool {
         match self {
-            Nullability::NonNullable => None,
-            Nullability::Nullable => Some(t),
+            Nullability::NonNullable => false,
+            Nullability::Nullable => true,
         }
     }
 }

vortex-vector/src/bool/vector_mut.rs

@@ -8,7 +8,7 @@ use vortex_dtype::{DType, Nullability};
 use vortex_mask::MaskMut;
 
 use super::BoolVector;
-use crate::VectorMutOps;
+use crate::{VectorMutOps, VectorOps};
 
 /// A mutable vector of boolean values.
 ///
@@ -25,7 +25,9 @@ pub struct BoolVectorMut {
 impl BoolVectorMut {
     /// Creates a new mutable boolean vector with the given `capacity` and `nullability`.
     pub fn with_capacity(capacity: usize, nullability: Nullability) -> Self {
-        let validity = nullability.is_nullable_then(|| MaskMut::with_capacity(capacity));
+        let validity = nullability
+            .is_nullable()
+            .then(|| MaskMut::with_capacity(capacity));
 
         Self {
             bits: BitBufferMut::with_capacity(capacity),
@@ -68,9 +70,13 @@ impl VectorMutOps for BoolVectorMut {
     }
 
     fn extend_from_vector(&mut self, other: &BoolVector) {
+        assert!(
+            self.is_nullable() || !other.is_nullable(),
+            "tried to extend a non-nullable `BoolVector` with a nullable vector"
+        );
+
         self.bits.append_buffer(&other.bits);
 
-        // TODO(connor): We must `other`'s nullability in relation to `self`.
         match (&mut self.validity, &other.validity) {
             (Some(self_v), Some(other_v)) => self_v.append_mask(other_v),
             (Some(self_v), None) => self_v.append_n(true, other.bits.len()),

vortex-vector/src/ops.rs

@@ -13,6 +13,13 @@ pub trait VectorOps: private::Sealed + Into<Vector> {
     /// Returns the [`Nullability`] of the vector.
     fn nullability(&self) -> Nullability;
 
+    /// Returns `true` if the nullability of this vector is [`Nullable`].
+    ///
+    /// [`Nullable`]: Nullability::Nullable
+    fn is_nullable(&self) -> bool {
+        self.nullability().is_nullable()
+    }
+
     /// Returns the [`DType`] (or data type) of the vector.
     fn dtype(&self) -> DType;
 
@@ -46,6 +53,13 @@ pub trait VectorMutOps: private::Sealed + Into<VectorMut> {
     /// Returns the [`Nullability`] of the vector.
     fn nullability(&self) -> Nullability;
 
+    /// Returns `true` if the nullability of this vector is [`Nullable`].
+    ///
+    /// [`Nullable`]: Nullability::Nullable
+    fn is_nullable(&self) -> bool {
+        self.nullability().is_nullable()
+    }
+
     /// Returns the [`DType`] (or data type) of the vector.
     fn dtype(&self) -> DType;
 
@@ -72,8 +86,10 @@ pub trait VectorMutOps: private::Sealed + Into<VectorMut> {
 
     /// Extends the vector by appending elements from another vector.
     ///
-    /// TODO(connor): Document semantics of what happens if `self` is non-nullable and `other` is
-    /// nullable (should panic?).
+    /// # Panics
+    ///
+    /// If `self` is a non-nullable vector, and `other` is a nullable vector, implementors should
+    /// ensure that this function panics.
     fn extend_from_vector(&mut self, other: &Self::Immutable);
 
     /// Converts `self` into an immutable vector.
@@ -93,6 +109,7 @@ pub trait VectorMutOps: private::Sealed + Into<VectorMut> {
     /// `at > capacity`).
     fn split_off(&mut self, at: usize) -> Self;
 
+    // TODO(connor): Should this panic if other has a different nullability?
     /// Absorbs a mutable vector that was previously split off.
     ///
     /// If the two vectors were previously contiguous and not mutated in a way that causes

vortex-vector/src/primitive/generic_mut.rs

@@ -7,7 +7,7 @@ use vortex_buffer::BufferMut;
 use vortex_dtype::{DType, NativePType, Nullability};
 use vortex_mask::MaskMut;
 
-use crate::{GenericPVector, VectorMutOps};
+use crate::{GenericPVector, VectorMutOps, VectorOps};
 
 /// A mutable vector of generic primitive values.
 ///
@@ -25,7 +25,9 @@ pub struct GenericPVectorMut<T> {
 impl<T: NativePType> GenericPVectorMut<T> {
     /// Create a new mutable primitive vector with the given capacity and nullability.
     pub fn with_capacity(capacity: usize, nullability: Nullability) -> Self {
-        let validity = nullability.is_nullable_then(|| MaskMut::with_capacity(capacity));
+        let validity = nullability
+            .is_nullable()
+            .then(|| MaskMut::with_capacity(capacity));
 
         Self {
             elements: BufferMut::with_capacity(capacity),
@@ -62,7 +64,13 @@ impl<T: NativePType> VectorMutOps for GenericPVectorMut<T> {
 
     /// Extends the vector by appending elements from another vector.
     fn extend_from_vector(&mut self, other: &GenericPVector<T>) {
+        assert!(
+            self.is_nullable() || !other.is_nullable(),
+            "tried to extend a non-nullable `GenericPVector` with a nullable vector"
+        );
+
         self.elements.extend_from_slice(other.elements.as_slice());
+
         match (&mut self.validity, &other.validity) {
             (Some(self_v), Some(other_v)) => self_v.append_mask(other_v),
             (Some(self_v), None) => self_v.append_n(true, other.elements.len()),