You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/05/24 13:53:36 UTC
[PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
alamb opened a new pull request, #5798:
URL: https://github.com/apache/arrow-rs/pull/5798
# Which issue does this PR close?
N/A
# Rationale for this change
I ran into someone at a conference yesterday who was saying they hadn't been
able to figure out how to modify arrays in place. @tustvold pointed out to me it
is possible but the documentation for `binary_mut` and `unary_mut` was
not as clear as it could be,
I had the code up for https://github.com/apache/arrow-rs/pull/5792 anyways so I figured I would bang out this improvement as well
# What changes are included in this PR?
1. Update documentation and add examples showing how this would work
This ended up being larger than I expected as I opened the docs and was imaginging "how would a random person who didn't understand the details of arrow-rs use this feature"
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
Doc changes only, no code changes
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1619635642
##########
arrow-arith/src/arity.rs:
##########
@@ -207,23 +227,46 @@ where
Ok(PrimitiveArray::new(buffer.into(), nulls))
}
-/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in `0..len`, mutating
-/// the mutable [`PrimitiveArray`] `a`. If any index is null in either `a` or `b`, the
-/// corresponding index in the result will also be null.
+/// Applies a binary and infallible function to values in two arrays, replacing
+/// the values in the first array in place.
///
-/// Mutable primitive array means that the buffer is not shared with other arrays.
-/// As a result, this mutates the buffer directly without allocating new buffer.
+/// # Details
+///
+/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in
+/// `0..len`, modifying the [`PrimitiveArray`] `a` in place, if possible.
+///
+/// If any index is null in either `a` or `b`, the corresponding index in the
+/// result will also be null.
+///
+/// # Buffer Reuse
+///
+/// If the underlying buffers in `a` are not shared with other arrays, mutates
+/// the underlying buffer in place, without allocating.
///
/// Like [`unary`] the provided function is evaluated for every index, ignoring validity. This
/// is beneficial when the cost of the operation is low compared to the cost of branching, and
/// especially when the operation can be vectorised, however, requires `op` to be infallible
/// for all possible values of its inputs
///
-/// # Error
+/// # Errors
///
-/// This function gives error if the arrays have different lengths.
-/// This function gives error of original [`PrimitiveArray`] `a` if it is not a mutable
-/// primitive array.
+/// * if the arrays have different lengths.
+///
+/// # See Also
+///
+/// * Documentation on [`PrimitiveArray::unary_mut`] for operating on [`ArrayRef`].
+///
+/// # Example
+/// ```
+/// # use arrow_arith::arity::binary_mut;
+/// # use arrow_array::Float32Array;
+/// # use arrow_array::types::Int32Type;
+/// let a = Float32Array::from(vec![Some(5.1f32), None, Some(6.8)]);
+/// let b = Float32Array::from(vec![Some(1.0f32), None, Some(2.0)]);
+/// // compute a + b, updating the value in a in place if possible
+/// let a = binary_mut(a, &b, |a, b| a + b).unwrap().unwrap();
Review Comment:
I filed a follow on ticket to track https://github.com/apache/arrow-rs/issues/5818
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613554864
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -915,8 +964,16 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
PrimitiveArray::new(values, Some(nulls))
}
- /// Returns `PrimitiveBuilder` of this primitive array for mutating its values if the underlying
- /// data buffer is not shared by others.
+ /// Returns a `PrimitiveBuilder` for this array, suitable for mutating values
+ /// in place.
+ ///
+ /// # Buffer Reuse
+ ///
+ /// If the underlying data buffer has no other outstanding references, the
+ /// buffer is used without copying.
+ ///
+ /// If the underlying data buffer does have outstanding references, the
+ /// buffer is cloned.
Review Comment:
This is not true, if the buffers can't be cloned it returns `Err(Self)`, much like https://doc.rust-lang.org/std/sync/struct.Arc.html#method.try_unwrap
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1629342361
##########
arrow-arith/src/arity.rs:
##########
@@ -207,23 +227,46 @@ where
Ok(PrimitiveArray::new(buffer.into(), nulls))
}
-/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in `0..len`, mutating
-/// the mutable [`PrimitiveArray`] `a`. If any index is null in either `a` or `b`, the
-/// corresponding index in the result will also be null.
+/// Applies a binary and infallible function to values in two arrays, replacing
+/// the values in the first array in place.
///
-/// Mutable primitive array means that the buffer is not shared with other arrays.
-/// As a result, this mutates the buffer directly without allocating new buffer.
+/// # Details
+///
+/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in
+/// `0..len`, modifying the [`PrimitiveArray`] `a` in place, if possible.
+///
+/// If any index is null in either `a` or `b`, the corresponding index in the
+/// result will also be null.
+///
+/// # Buffer Reuse
+///
+/// If the underlying buffers in `a` are not shared with other arrays, mutates
+/// the underlying buffer in place, without allocating.
///
/// Like [`unary`] the provided function is evaluated for every index, ignoring validity. This
/// is beneficial when the cost of the operation is low compared to the cost of branching, and
/// especially when the operation can be vectorised, however, requires `op` to be infallible
/// for all possible values of its inputs
///
-/// # Error
+/// # Errors
///
-/// This function gives error if the arrays have different lengths.
-/// This function gives error of original [`PrimitiveArray`] `a` if it is not a mutable
-/// primitive array.
+/// * if the arrays have different lengths.
+///
+/// # See Also
+///
+/// * Documentation on [`PrimitiveArray::unary_mut`] for operating on [`ArrayRef`].
+///
+/// # Example
+/// ```
+/// # use arrow_arith::arity::binary_mut;
+/// # use arrow_array::Float32Array;
+/// # use arrow_array::types::Int32Type;
+/// let a = Float32Array::from(vec![Some(5.1f32), None, Some(6.8)]);
+/// let b = Float32Array::from(vec![Some(1.0f32), None, Some(2.0)]);
+/// // compute a + b, updating the value in a in place if possible
+/// let a = binary_mut(a, &b, |a, b| a + b).unwrap().unwrap();
Review Comment:
Update is that @viirya fixed this in https://github.com/apache/arrow-rs/pull/5833
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1619659066
##########
arrow-arith/src/arity.rs:
##########
@@ -207,23 +227,46 @@ where
Ok(PrimitiveArray::new(buffer.into(), nulls))
}
-/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in `0..len`, mutating
-/// the mutable [`PrimitiveArray`] `a`. If any index is null in either `a` or `b`, the
-/// corresponding index in the result will also be null.
+/// Applies a binary and infallible function to values in two arrays, replacing
+/// the values in the first array in place.
///
-/// Mutable primitive array means that the buffer is not shared with other arrays.
-/// As a result, this mutates the buffer directly without allocating new buffer.
+/// # Details
+///
+/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in
+/// `0..len`, modifying the [`PrimitiveArray`] `a` in place, if possible.
+///
+/// If any index is null in either `a` or `b`, the corresponding index in the
+/// result will also be null.
+///
+/// # Buffer Reuse
+///
+/// If the underlying buffers in `a` are not shared with other arrays, mutates
+/// the underlying buffer in place, without allocating.
///
/// Like [`unary`] the provided function is evaluated for every index, ignoring validity. This
/// is beneficial when the cost of the operation is low compared to the cost of branching, and
/// especially when the operation can be vectorised, however, requires `op` to be infallible
/// for all possible values of its inputs
///
-/// # Error
+/// # Errors
///
-/// This function gives error if the arrays have different lengths.
-/// This function gives error of original [`PrimitiveArray`] `a` if it is not a mutable
-/// primitive array.
+/// * if the arrays have different lengths.
+///
+/// # See Also
+///
+/// * Documentation on [`PrimitiveArray::unary_mut`] for operating on [`ArrayRef`].
+///
+/// # Example
+/// ```
+/// # use arrow_arith::arity::binary_mut;
+/// # use arrow_array::Float32Array;
+/// # use arrow_array::types::Int32Type;
+/// let a = Float32Array::from(vec![Some(5.1f32), None, Some(6.8)]);
+/// let b = Float32Array::from(vec![Some(1.0f32), None, Some(2.0)]);
+/// // compute a + b, updating the value in a in place if possible
+/// let a = binary_mut(a, &b, |a, b| a + b).unwrap().unwrap();
Review Comment:
I think it can be different types. Not intentional, but like @tustvold said.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1617594683
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -419,7 +419,7 @@ pub type Decimal256Array = PrimitiveArray<Decimal256Type>;
pub use crate::types::ArrowPrimitiveType;
-/// An array of [primitive values](https://arrow.apache.org/docs/format/Columnar.html#fixed-size-primitive-layout)
+/// An array of primtive values, of type [`ArrowPrimitiveType`]
Review Comment:
```suggestion
/// An array of primitive values, of type [`ArrowPrimitiveType`]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1620849230
##########
arrow-arith/src/arity.rs:
##########
@@ -207,23 +227,46 @@ where
Ok(PrimitiveArray::new(buffer.into(), nulls))
}
-/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in `0..len`, mutating
-/// the mutable [`PrimitiveArray`] `a`. If any index is null in either `a` or `b`, the
-/// corresponding index in the result will also be null.
+/// Applies a binary and infallible function to values in two arrays, replacing
+/// the values in the first array in place.
///
-/// Mutable primitive array means that the buffer is not shared with other arrays.
-/// As a result, this mutates the buffer directly without allocating new buffer.
+/// # Details
+///
+/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in
+/// `0..len`, modifying the [`PrimitiveArray`] `a` in place, if possible.
+///
+/// If any index is null in either `a` or `b`, the corresponding index in the
+/// result will also be null.
+///
+/// # Buffer Reuse
+///
+/// If the underlying buffers in `a` are not shared with other arrays, mutates
+/// the underlying buffer in place, without allocating.
///
/// Like [`unary`] the provided function is evaluated for every index, ignoring validity. This
/// is beneficial when the cost of the operation is low compared to the cost of branching, and
/// especially when the operation can be vectorised, however, requires `op` to be infallible
/// for all possible values of its inputs
///
-/// # Error
+/// # Errors
///
-/// This function gives error if the arrays have different lengths.
-/// This function gives error of original [`PrimitiveArray`] `a` if it is not a mutable
-/// primitive array.
+/// * if the arrays have different lengths.
+///
+/// # See Also
+///
+/// * Documentation on [`PrimitiveArray::unary_mut`] for operating on [`ArrayRef`].
+///
+/// # Example
+/// ```
+/// # use arrow_arith::arity::binary_mut;
+/// # use arrow_array::Float32Array;
+/// # use arrow_array::types::Int32Type;
+/// let a = Float32Array::from(vec![Some(5.1f32), None, Some(6.8)]);
+/// let b = Float32Array::from(vec![Some(1.0f32), None, Some(2.0)]);
+/// // compute a + b, updating the value in a in place if possible
+/// let a = binary_mut(a, &b, |a, b| a + b).unwrap().unwrap();
Review Comment:
Cool -- thanks @viirya -- I don't think there is any particular hurry or urgency
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613555382
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -766,23 +791,43 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
PrimitiveArray::new(buffer.into(), nulls)
}
- /// Applies an unary and infallible function to a mutable primitive array.
- /// Mutable primitive array means that the buffer is not shared with other arrays.
- /// As a result, this mutates the buffer directly without allocating new buffer.
+ /// Applies a unary and infallible function to the array in place if possible.
+ ///
+ /// # Buffer Reuse
+ ///
+ /// If the underlying buffers are not shared with other arrays, mutates the
+ /// underlying buffer in place, without allocating.
+ ///
+ /// # Null Handling
///
- /// # Implementation
+ /// See [`Self::unary`] for more information on null handling.
///
- /// This will apply the function for all values, including those on null slots.
- /// This implies that the operation must be infallible for any value of the corresponding type
- /// or this function may panic.
/// # Example
+ ///
/// ```rust
/// # use arrow_array::{Int32Array, types::Int32Type};
- /// # fn main() {
/// let array = Int32Array::from(vec![Some(5), Some(7), None]);
+ /// // Apply x*2+1 to the data in place, no allocations
+ /// let c = array.unary_mut(|x| x * 2 + 1).unwrap();
+ /// assert_eq!(c, Int32Array::from(vec![Some(11), Some(15), None]));
+ /// ```
+ ///
+ /// # Example: modify [`ArrayRef`] in place, if not shared
+ ///
+ /// It is also possible to modify an [`ArrayRef`] if there are no other
+ /// references to the underlying buffer.
+ ///
+ /// ```rust
+ /// # use std::sync::Arc;
+ /// # use arrow_array::{Array, ArrayRef, Int32Array, PrimitiveArray, types::Int32Type};
+ /// # let array: ArrayRef = Arc::new(Int32Array::from(vec![Some(5), Some(7), None]));
+ /// // Convert to Int32Array (panic's if array.data_type is not Int32)
+ /// let array = PrimitiveArray::<Int32Type>::from(array.into_data());
+ /// // Apply x*2+1 to the data in place, no allocations if
+ /// // there are no other references to the underlying buffer
+ /// // will create a new buffer if there are references.
Review Comment:
This is not true, it returns `Err(Self)` if there are outstanding references
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613551691
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -480,6 +480,19 @@ pub use crate::types::ArrowPrimitiveType;
/// assert_eq!(array.values(), &[1, 0, 2]);
/// assert!(array.is_null(1));
/// ```
+///
+/// # Example: Get a `PrimitiveArray` from an [`ArrayRef`]
+/// ```
+/// # use std::sync::Arc;
+/// # use arrow_array::{Array, ArrayRef, Float32Array, PrimitiveArray};
+/// # use arrow_array::types::{Float32Type};
+/// # use arrow_schema::DataType;
+/// # let array: ArrayRef = Arc::new(Float32Array::from(vec![1.2, 2.3]));
+/// // will panic if the array is not a Float32Array
+/// assert_eq!(&DataType::Float32, array.data_type());
+/// let f32_array = PrimitiveArray::<Float32Type>::from(array.into_data());
Review Comment:
It is perhaps worth highlighting that `into_data` will probably be slower than just downcasting the array, cloning it, and then dropping the ArrayRef
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613556901
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -480,6 +480,19 @@ pub use crate::types::ArrowPrimitiveType;
/// assert_eq!(array.values(), &[1, 0, 2]);
/// assert!(array.is_null(1));
/// ```
+///
+/// # Example: Get a `PrimitiveArray` from an [`ArrayRef`]
+/// ```
+/// # use std::sync::Arc;
+/// # use arrow_array::{Array, ArrayRef, Float32Array, PrimitiveArray};
+/// # use arrow_array::types::{Float32Type};
+/// # use arrow_schema::DataType;
+/// # let array: ArrayRef = Arc::new(Float32Array::from(vec![1.2, 2.3]));
+/// // will panic if the array is not a Float32Array
+/// assert_eq!(&DataType::Float32, array.data_type());
+/// let f32_array = PrimitiveArray::<Float32Type>::from(array.into_data());
Review Comment:
I have no idea to be honest -- if downcasting / cloning / dropping is faster I will update the docs to do that.
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -766,23 +787,43 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
PrimitiveArray::new(buffer.into(), nulls)
}
- /// Applies an unary and infallible function to a mutable primitive array.
- /// Mutable primitive array means that the buffer is not shared with other arrays.
- /// As a result, this mutates the buffer directly without allocating new buffer.
+ /// Applies a unary and infallible function to the array in place if possible.
+ ///
+ /// # Buffer Reuse
+ ///
+ /// If the underlying buffers are not shared with other arrays, mutates the
+ /// underlying buffer in place, without allocating a new buffer.
+ ///
+ /// # Null Handling
///
- /// # Implementation
+ /// See [`Self::unary`] for more information on null handling.
///
- /// This will apply the function for all values, including those on null slots.
- /// This implies that the operation must be infallible for any value of the corresponding type
- /// or this function may panic.
/// # Example
+ ///
/// ```rust
/// # use arrow_array::{Int32Array, types::Int32Type};
- /// # fn main() {
/// let array = Int32Array::from(vec![Some(5), Some(7), None]);
+ /// // Apply x*2+1 to the data in place, no allocations
+ /// let c = array.unary_mut(|x| x * 2 + 1).unwrap();
+ /// assert_eq!(c, Int32Array::from(vec![Some(11), Some(15), None]));
+ /// ```
+ ///
+ /// # Example: modify [`ArrayRef`] in place, if not shared
Review Comment:
This is the example I imagine someone might be looking for
##########
arrow-arith/src/arity.rs:
##########
@@ -207,23 +227,46 @@ where
Ok(PrimitiveArray::new(buffer.into(), nulls))
}
-/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in `0..len`, mutating
-/// the mutable [`PrimitiveArray`] `a`. If any index is null in either `a` or `b`, the
-/// corresponding index in the result will also be null.
+/// Applies a binary and infallible function to values in two arrays, replacing
+/// the values in the first array in place.
///
-/// Mutable primitive array means that the buffer is not shared with other arrays.
-/// As a result, this mutates the buffer directly without allocating new buffer.
+/// # Details
+///
+/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in
+/// `0..len`, modifying the [`PrimitiveArray`] `a` in place, if possible.
+///
+/// If any index is null in either `a` or `b`, the corresponding index in the
+/// result will also be null.
+///
+/// # Buffer Reuse
+///
+/// If the underlying buffers in `a` are not shared with other arrays, mutates
+/// the underlying buffer in place, without allocating.
///
/// Like [`unary`] the provided function is evaluated for every index, ignoring validity. This
/// is beneficial when the cost of the operation is low compared to the cost of branching, and
/// especially when the operation can be vectorised, however, requires `op` to be infallible
/// for all possible values of its inputs
///
-/// # Error
+/// # Errors
///
-/// This function gives error if the arrays have different lengths.
-/// This function gives error of original [`PrimitiveArray`] `a` if it is not a mutable
-/// primitive array.
+/// * if the arrays have different lengths.
+///
+/// # See Also
+///
+/// * Documentation on [`PrimitiveArray::unary_mut`] for operating on [`ArrayRef`].
+///
+/// # Example
+/// ```
+/// # use arrow_arith::arity::binary_mut;
+/// # use arrow_array::Float32Array;
+/// # use arrow_array::types::Int32Type;
+/// let a = Float32Array::from(vec![Some(5.1f32), None, Some(6.8)]);
+/// let b = Float32Array::from(vec![Some(1.0f32), None, Some(2.0)]);
+/// // compute a + b, updating the value in a in place if possible
+/// let a = binary_mut(a, &b, |a, b| a + b).unwrap().unwrap();
Review Comment:
I found this API inconsistent for two reasons:
1. It requires both `a` and `b` to be the same type of array (where `binary` can take different types)
2. It returns a wrapped `Result` (`Result<Result<..>>`).
@viirya I think you added this API in https://github.com/apache/arrow-rs/pull/3144
Was it intentional to have these features?
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -732,22 +745,30 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
PrimitiveArray::from(unsafe { d.build_unchecked() })
}
- /// Applies an unary and infallible function to a primitive array.
- /// This is the fastest way to perform an operation on a primitive array when
- /// the benefits of a vectorized operation outweigh the cost of branching nulls and non-nulls.
+ /// Applies a unary infallible function to a primitive array, producing a
+ /// new array of potentially different type.
+ ///
+ /// This is the fastest way to perform an operation on a primitive array
+ /// when the benefits of a vectorized operation outweigh the cost of
+ /// branching nulls and non-nulls.
///
- /// # Implementation
+ /// See [`Self::unary_mut`] for in place modification.
+ ///
+ /// # Null Handling
+ ///
+ /// Applies the function for all values, including those on null slots. This
+ /// will often allow the compiler to generate faster vectorized code, but
+ /// requires that the operation must be infallible (not error/panic) for any
+ /// value of the corresponding type or this function may panic.
///
- /// This will apply the function for all values, including those on null slots.
- /// This implies that the operation must be infallible for any value of the corresponding type
- /// or this function may panic.
/// # Example
/// ```rust
- /// # use arrow_array::{Int32Array, types::Int32Type};
+ /// # use arrow_array::{Int32Array, Float32Array, types::Int32Type};
/// # fn main() {
/// let array = Int32Array::from(vec![Some(5), Some(7), None]);
- /// let c = array.unary(|x| x * 2 + 1);
- /// assert_eq!(c, Int32Array::from(vec![Some(11), Some(15), None]));
+ /// // Create a new array with the value of applying sqrt
Review Comment:
changed this to show you can make a different type
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -480,6 +480,19 @@ pub use crate::types::ArrowPrimitiveType;
/// assert_eq!(array.values(), &[1, 0, 2]);
/// assert!(array.is_null(1));
/// ```
+///
+/// # Example: Get a `PrimitiveArray` from an [`ArrayRef`]
Review Comment:
I don't think it is obvious how to go from `Arc<dyn Array>` back to `PrimitiveArray` so I documented that too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613582371
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -915,8 +964,16 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
PrimitiveArray::new(values, Some(nulls))
}
- /// Returns `PrimitiveBuilder` of this primitive array for mutating its values if the underlying
- /// data buffer is not shared by others.
+ /// Returns a `PrimitiveBuilder` for this array, suitable for mutating values
+ /// in place.
+ ///
+ /// # Buffer Reuse
+ ///
+ /// If the underlying data buffer has no other outstanding references, the
+ /// buffer is used without copying.
+ ///
+ /// If the underlying data buffer does have outstanding references, the
+ /// buffer is cloned.
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "wiedld (via GitHub)" <gi...@apache.org>.
wiedld commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1617540834
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -419,7 +419,7 @@ pub type Decimal256Array = PrimitiveArray<Decimal256Type>;
pub use crate::types::ArrowPrimitiveType;
-/// An array of [primitive values](https://arrow.apache.org/docs/format/Columnar.html#fixed-size-primitive-layout)
+/// An array of primtive values, of type [`ArrowPrimitiveType`]
Review Comment:
nit: misspelling. `primtive`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1619660269
##########
arrow-arith/src/arity.rs:
##########
@@ -207,23 +227,46 @@ where
Ok(PrimitiveArray::new(buffer.into(), nulls))
}
-/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in `0..len`, mutating
-/// the mutable [`PrimitiveArray`] `a`. If any index is null in either `a` or `b`, the
-/// corresponding index in the result will also be null.
+/// Applies a binary and infallible function to values in two arrays, replacing
+/// the values in the first array in place.
///
-/// Mutable primitive array means that the buffer is not shared with other arrays.
-/// As a result, this mutates the buffer directly without allocating new buffer.
+/// # Details
+///
+/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in
+/// `0..len`, modifying the [`PrimitiveArray`] `a` in place, if possible.
+///
+/// If any index is null in either `a` or `b`, the corresponding index in the
+/// result will also be null.
+///
+/// # Buffer Reuse
+///
+/// If the underlying buffers in `a` are not shared with other arrays, mutates
+/// the underlying buffer in place, without allocating.
///
/// Like [`unary`] the provided function is evaluated for every index, ignoring validity. This
/// is beneficial when the cost of the operation is low compared to the cost of branching, and
/// especially when the operation can be vectorised, however, requires `op` to be infallible
/// for all possible values of its inputs
///
-/// # Error
+/// # Errors
///
-/// This function gives error if the arrays have different lengths.
-/// This function gives error of original [`PrimitiveArray`] `a` if it is not a mutable
-/// primitive array.
+/// * if the arrays have different lengths.
+///
+/// # See Also
+///
+/// * Documentation on [`PrimitiveArray::unary_mut`] for operating on [`ArrayRef`].
+///
+/// # Example
+/// ```
+/// # use arrow_arith::arity::binary_mut;
+/// # use arrow_array::Float32Array;
+/// # use arrow_array::types::Int32Type;
+/// let a = Float32Array::from(vec![Some(5.1f32), None, Some(6.8)]);
+/// let b = Float32Array::from(vec![Some(1.0f32), None, Some(2.0)]);
+/// // compute a + b, updating the value in a in place if possible
+/// let a = binary_mut(a, &b, |a, b| a + b).unwrap().unwrap();
Review Comment:
It's not intentional but just for the usage at the moment. It could be more general to have different types. I can open a PR to do it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#issuecomment-2148379184
I plan to merge this PR in the next day or two unless there is any additional commentary. It adds test coverage and changes docs -- if we find issues or additional improvements we can do them as follow on PRs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613527391
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -766,23 +787,43 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
PrimitiveArray::new(buffer.into(), nulls)
}
- /// Applies an unary and infallible function to a mutable primitive array.
- /// Mutable primitive array means that the buffer is not shared with other arrays.
- /// As a result, this mutates the buffer directly without allocating new buffer.
+ /// Applies a unary and infallible function to the array in place if possible.
+ ///
+ /// # Buffer Reuse
+ ///
+ /// If the underlying buffers are not shared with other arrays, mutates the
+ /// underlying buffer in place, without allocating a new buffer.
+ ///
+ /// # Null Handling
///
- /// # Implementation
+ /// See [`Self::unary`] for more information on null handling.
///
- /// This will apply the function for all values, including those on null slots.
- /// This implies that the operation must be infallible for any value of the corresponding type
- /// or this function may panic.
/// # Example
+ ///
/// ```rust
/// # use arrow_array::{Int32Array, types::Int32Type};
- /// # fn main() {
/// let array = Int32Array::from(vec![Some(5), Some(7), None]);
+ /// // Apply x*2+1 to the data in place, no allocations
+ /// let c = array.unary_mut(|x| x * 2 + 1).unwrap();
+ /// assert_eq!(c, Int32Array::from(vec![Some(11), Some(15), None]));
+ /// ```
+ ///
+ /// # Example: modify [`ArrayRef`] in place, if not shared
Review Comment:
This is an example I imagine someone might be looking for (like I would be)
##########
arrow-arith/src/arity.rs:
##########
@@ -207,23 +227,46 @@ where
Ok(PrimitiveArray::new(buffer.into(), nulls))
}
-/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in `0..len`, mutating
-/// the mutable [`PrimitiveArray`] `a`. If any index is null in either `a` or `b`, the
-/// corresponding index in the result will also be null.
+/// Applies a binary and infallible function to values in two arrays, replacing
+/// the values in the first array in place.
///
-/// Mutable primitive array means that the buffer is not shared with other arrays.
-/// As a result, this mutates the buffer directly without allocating new buffer.
+/// # Details
+///
+/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in
+/// `0..len`, modifying the [`PrimitiveArray`] `a` in place, if possible.
+///
+/// If any index is null in either `a` or `b`, the corresponding index in the
+/// result will also be null.
+///
+/// # Buffer Reuse
+///
+/// If the underlying buffers in `a` are not shared with other arrays, mutates
+/// the underlying buffer in place, without allocating.
///
/// Like [`unary`] the provided function is evaluated for every index, ignoring validity. This
/// is beneficial when the cost of the operation is low compared to the cost of branching, and
/// especially when the operation can be vectorised, however, requires `op` to be infallible
/// for all possible values of its inputs
///
-/// # Error
+/// # Errors
///
-/// This function gives error if the arrays have different lengths.
-/// This function gives error of original [`PrimitiveArray`] `a` if it is not a mutable
-/// primitive array.
+/// * if the arrays have different lengths.
+///
+/// # See Also
+///
+/// * Documentation on [`PrimitiveArray::unary_mut`] for operating on [`ArrayRef`].
+///
+/// # Example
+/// ```
+/// # use arrow_arith::arity::binary_mut;
+/// # use arrow_array::Float32Array;
+/// # use arrow_array::types::Int32Type;
+/// let a = Float32Array::from(vec![Some(5.1f32), None, Some(6.8)]);
+/// let b = Float32Array::from(vec![Some(1.0f32), None, Some(2.0)]);
+/// // compute a + b, updating the value in a in place if possible
+/// let a = binary_mut(a, &b, |a, b| a + b).unwrap().unwrap();
Review Comment:
Added an example showing how to use the wrapped errors. Will wait for @viirya to comment on the types
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#issuecomment-2138462556
I think this PR is ready for review
I have filed https://github.com/apache/arrow-rs/issues/5770 to track the "binary_mut" type check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613559065
##########
arrow-arith/src/arity.rs:
##########
@@ -207,23 +227,46 @@ where
Ok(PrimitiveArray::new(buffer.into(), nulls))
}
-/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in `0..len`, mutating
-/// the mutable [`PrimitiveArray`] `a`. If any index is null in either `a` or `b`, the
-/// corresponding index in the result will also be null.
+/// Applies a binary and infallible function to values in two arrays, replacing
+/// the values in the first array in place.
///
-/// Mutable primitive array means that the buffer is not shared with other arrays.
-/// As a result, this mutates the buffer directly without allocating new buffer.
+/// # Details
+///
+/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in
+/// `0..len`, modifying the [`PrimitiveArray`] `a` in place, if possible.
+///
+/// If any index is null in either `a` or `b`, the corresponding index in the
+/// result will also be null.
+///
+/// # Buffer Reuse
+///
+/// If the underlying buffers in `a` are not shared with other arrays, mutates
+/// the underlying buffer in place, without allocating.
///
/// Like [`unary`] the provided function is evaluated for every index, ignoring validity. This
/// is beneficial when the cost of the operation is low compared to the cost of branching, and
/// especially when the operation can be vectorised, however, requires `op` to be infallible
/// for all possible values of its inputs
///
-/// # Error
+/// # Errors
///
-/// This function gives error if the arrays have different lengths.
-/// This function gives error of original [`PrimitiveArray`] `a` if it is not a mutable
-/// primitive array.
+/// * if the arrays have different lengths.
+///
+/// # See Also
+///
+/// * Documentation on [`PrimitiveArray::unary_mut`] for operating on [`ArrayRef`].
+///
+/// # Example
+/// ```
+/// # use arrow_arith::arity::binary_mut;
+/// # use arrow_array::Float32Array;
+/// # use arrow_array::types::Int32Type;
+/// let a = Float32Array::from(vec![Some(5.1f32), None, Some(6.8)]);
+/// let b = Float32Array::from(vec![Some(1.0f32), None, Some(2.0)]);
+/// // compute a + b, updating the value in a in place if possible
+/// let a = binary_mut(a, &b, |a, b| a + b).unwrap().unwrap();
Review Comment:
I think I see it is the same thing @tustvold points out in https://github.com/apache/arrow-rs/pull/5798/files#r1613555382 -- I will fix
##########
arrow-arith/src/arity.rs:
##########
@@ -207,23 +227,46 @@ where
Ok(PrimitiveArray::new(buffer.into(), nulls))
}
-/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in `0..len`, mutating
-/// the mutable [`PrimitiveArray`] `a`. If any index is null in either `a` or `b`, the
-/// corresponding index in the result will also be null.
+/// Applies a binary and infallible function to values in two arrays, replacing
+/// the values in the first array in place.
///
-/// Mutable primitive array means that the buffer is not shared with other arrays.
-/// As a result, this mutates the buffer directly without allocating new buffer.
+/// # Details
+///
+/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in
+/// `0..len`, modifying the [`PrimitiveArray`] `a` in place, if possible.
+///
+/// If any index is null in either `a` or `b`, the corresponding index in the
+/// result will also be null.
+///
+/// # Buffer Reuse
+///
+/// If the underlying buffers in `a` are not shared with other arrays, mutates
+/// the underlying buffer in place, without allocating.
///
/// Like [`unary`] the provided function is evaluated for every index, ignoring validity. This
/// is beneficial when the cost of the operation is low compared to the cost of branching, and
/// especially when the operation can be vectorised, however, requires `op` to be infallible
/// for all possible values of its inputs
///
-/// # Error
+/// # Errors
///
-/// This function gives error if the arrays have different lengths.
-/// This function gives error of original [`PrimitiveArray`] `a` if it is not a mutable
-/// primitive array.
+/// * if the arrays have different lengths.
+///
+/// # See Also
+///
+/// * Documentation on [`PrimitiveArray::unary_mut`] for operating on [`ArrayRef`].
+///
+/// # Example
+/// ```
+/// # use arrow_arith::arity::binary_mut;
+/// # use arrow_array::Float32Array;
+/// # use arrow_array::types::Int32Type;
+/// let a = Float32Array::from(vec![Some(5.1f32), None, Some(6.8)]);
+/// let b = Float32Array::from(vec![Some(1.0f32), None, Some(2.0)]);
+/// // compute a + b, updating the value in a in place if possible
+/// let a = binary_mut(a, &b, |a, b| a + b).unwrap().unwrap();
Review Comment:
I think I see it is the same thing @tustvold points out in https://github.com/apache/arrow-rs/pull/5798/files#r1613555382 -- I will document
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613560755
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -480,6 +480,19 @@ pub use crate::types::ArrowPrimitiveType;
/// assert_eq!(array.values(), &[1, 0, 2]);
/// assert!(array.is_null(1));
/// ```
+///
+/// # Example: Get a `PrimitiveArray` from an [`ArrayRef`]
+/// ```
+/// # use std::sync::Arc;
+/// # use arrow_array::{Array, ArrayRef, Float32Array, PrimitiveArray};
+/// # use arrow_array::types::{Float32Type};
+/// # use arrow_schema::DataType;
+/// # let array: ArrayRef = Arc::new(Float32Array::from(vec![1.2, 2.3]));
+/// // will panic if the array is not a Float32Array
+/// assert_eq!(&DataType::Float32, array.data_type());
+/// let f32_array = PrimitiveArray::<Float32Type>::from(array.into_data());
Review Comment:
It's likely marginal, but it avoids a fair amount of data marshalling so should be faster to not go via ArrayData
Edit: You could also use https://docs.rs/arrow-array/latest/arrow_array/cast/fn.downcast_array.html but that is just a wrapper around using into_data
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#issuecomment-2139739654
I believe CI is failing due to https://github.com/apache/arrow-rs/issues/5815
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613559421
##########
arrow-arith/src/arity.rs:
##########
@@ -207,23 +227,46 @@ where
Ok(PrimitiveArray::new(buffer.into(), nulls))
}
-/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in `0..len`, mutating
-/// the mutable [`PrimitiveArray`] `a`. If any index is null in either `a` or `b`, the
-/// corresponding index in the result will also be null.
+/// Applies a binary and infallible function to values in two arrays, replacing
+/// the values in the first array in place.
///
-/// Mutable primitive array means that the buffer is not shared with other arrays.
-/// As a result, this mutates the buffer directly without allocating new buffer.
+/// # Details
+///
+/// Given two arrays of length `len`, calls `op(a[i], b[i])` for `i` in
+/// `0..len`, modifying the [`PrimitiveArray`] `a` in place, if possible.
+///
+/// If any index is null in either `a` or `b`, the corresponding index in the
+/// result will also be null.
+///
+/// # Buffer Reuse
+///
+/// If the underlying buffers in `a` are not shared with other arrays, mutates
+/// the underlying buffer in place, without allocating.
///
/// Like [`unary`] the provided function is evaluated for every index, ignoring validity. This
/// is beneficial when the cost of the operation is low compared to the cost of branching, and
/// especially when the operation can be vectorised, however, requires `op` to be infallible
/// for all possible values of its inputs
///
-/// # Error
+/// # Errors
///
-/// This function gives error if the arrays have different lengths.
-/// This function gives error of original [`PrimitiveArray`] `a` if it is not a mutable
-/// primitive array.
+/// * if the arrays have different lengths.
+///
+/// # See Also
+///
+/// * Documentation on [`PrimitiveArray::unary_mut`] for operating on [`ArrayRef`].
+///
+/// # Example
+/// ```
+/// # use arrow_arith::arity::binary_mut;
+/// # use arrow_array::Float32Array;
+/// # use arrow_array::types::Int32Type;
+/// let a = Float32Array::from(vec![Some(5.1f32), None, Some(6.8)]);
+/// let b = Float32Array::from(vec![Some(1.0f32), None, Some(2.0)]);
+/// // compute a + b, updating the value in a in place if possible
+/// let a = binary_mut(a, &b, |a, b| a + b).unwrap().unwrap();
Review Comment:
The wrapped Result is necessary to distinguish between an error taking ownership and an error performing the computation.
As for the different types, that is likely just an oversight
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613557809
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -766,23 +791,43 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
PrimitiveArray::new(buffer.into(), nulls)
}
- /// Applies an unary and infallible function to a mutable primitive array.
- /// Mutable primitive array means that the buffer is not shared with other arrays.
- /// As a result, this mutates the buffer directly without allocating new buffer.
+ /// Applies a unary and infallible function to the array in place if possible.
+ ///
+ /// # Buffer Reuse
+ ///
+ /// If the underlying buffers are not shared with other arrays, mutates the
+ /// underlying buffer in place, without allocating.
+ ///
+ /// # Null Handling
///
- /// # Implementation
+ /// See [`Self::unary`] for more information on null handling.
///
- /// This will apply the function for all values, including those on null slots.
- /// This implies that the operation must be infallible for any value of the corresponding type
- /// or this function may panic.
/// # Example
+ ///
/// ```rust
/// # use arrow_array::{Int32Array, types::Int32Type};
- /// # fn main() {
/// let array = Int32Array::from(vec![Some(5), Some(7), None]);
+ /// // Apply x*2+1 to the data in place, no allocations
+ /// let c = array.unary_mut(|x| x * 2 + 1).unwrap();
+ /// assert_eq!(c, Int32Array::from(vec![Some(11), Some(15), None]));
+ /// ```
+ ///
+ /// # Example: modify [`ArrayRef`] in place, if not shared
+ ///
+ /// It is also possible to modify an [`ArrayRef`] if there are no other
+ /// references to the underlying buffer.
+ ///
+ /// ```rust
+ /// # use std::sync::Arc;
+ /// # use arrow_array::{Array, ArrayRef, Int32Array, PrimitiveArray, types::Int32Type};
+ /// # let array: ArrayRef = Arc::new(Int32Array::from(vec![Some(5), Some(7), None]));
+ /// // Convert to Int32Array (panic's if array.data_type is not Int32)
+ /// let array = PrimitiveArray::<Int32Type>::from(array.into_data());
+ /// // Apply x*2+1 to the data in place, no allocations if
+ /// // there are no other references to the underlying buffer
+ /// // will create a new buffer if there are references.
Review Comment:
Good call, will fix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613560755
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -480,6 +480,19 @@ pub use crate::types::ArrowPrimitiveType;
/// assert_eq!(array.values(), &[1, 0, 2]);
/// assert!(array.is_null(1));
/// ```
+///
+/// # Example: Get a `PrimitiveArray` from an [`ArrayRef`]
+/// ```
+/// # use std::sync::Arc;
+/// # use arrow_array::{Array, ArrayRef, Float32Array, PrimitiveArray};
+/// # use arrow_array::types::{Float32Type};
+/// # use arrow_schema::DataType;
+/// # let array: ArrayRef = Arc::new(Float32Array::from(vec![1.2, 2.3]));
+/// // will panic if the array is not a Float32Array
+/// assert_eq!(&DataType::Float32, array.data_type());
+/// let f32_array = PrimitiveArray::<Float32Type>::from(array.into_data());
Review Comment:
It's likely marginal, but it avoids a fair amount of data marshalling so should be faster to not go via ArrayData
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine documentation for `unary_mut` and `binary_mut` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5798:
URL: https://github.com/apache/arrow-rs/pull/5798#discussion_r1613588980
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -480,6 +480,19 @@ pub use crate::types::ArrowPrimitiveType;
/// assert_eq!(array.values(), &[1, 0, 2]);
/// assert!(array.is_null(1));
/// ```
+///
+/// # Example: Get a `PrimitiveArray` from an [`ArrayRef`]
+/// ```
+/// # use std::sync::Arc;
+/// # use arrow_array::{Array, ArrayRef, Float32Array, PrimitiveArray};
+/// # use arrow_array::types::{Float32Type};
+/// # use arrow_schema::DataType;
+/// # let array: ArrayRef = Arc::new(Float32Array::from(vec![1.2, 2.3]));
+/// // will panic if the array is not a Float32Array
+/// assert_eq!(&DataType::Float32, array.data_type());
+/// let f32_array = PrimitiveArray::<Float32Type>::from(array.into_data());
Review Comment:
Updated to use clone. I think that is the most common pattern. 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org