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

Refine documentation for unary_mut and binary_mut #5798

Merged
merged 5 commits into from
Jun 6, 2024

Conversation

alamb
Copy link
Contributor

@alamb alamb commented May 24, 2024

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 #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"

Are there any user-facing changes?

Doc changes only, no code changes

@github-actions github-actions bot added the arrow Changes to the arrow crate label May 24, 2024
@@ -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`]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is obvious how to go from Arc<dyn Array> back to PrimitiveArray so I documented that too

/// # 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed this to show you can make a different type

/// assert_eq!(c, Int32Array::from(vec![Some(11), Some(15), None]));
/// ```
///
/// # Example: modify [`ArrayRef`] in place, if not shared
Copy link
Contributor Author

@alamb alamb May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an example I imagine someone might be looking for (like I would be)

arrow-arith/src/arity.rs Outdated Show resolved Hide resolved
arrow-array/src/array/primitive_array.rs Outdated Show resolved Hide resolved
@alamb alamb marked this pull request as ready for review May 24, 2024 14:45
@alamb
Copy link
Contributor Author

alamb commented May 30, 2024

I think this PR is ready for review

I have filed #5770 to track the "binary_mut" type check

@alamb
Copy link
Contributor Author

alamb commented May 30, 2024

I believe CI is failing due to #5815

@alamb
Copy link
Contributor Author

alamb commented Jun 4, 2024

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

@alamb alamb merged commit 087f34b into apache:master Jun 6, 2024
24 of 25 checks passed
@alamb alamb added the documentation Improvements or additions to documentation label Jun 6, 2024
@alamb alamb deleted the alamb/op_docs branch June 6, 2024 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants