ARROW-10639: [Rust] Added examples to is_null kernel and simplified signature. #8701
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Any thoughts, @alamb , @nevi-me , @andygrove , since the requirement of |
vertexclique
approved these changes
Nov 18, 2020
Comment on lines
+115
to
+118
| /// # Error | ||
| /// This function never errors. | ||
| /// # Example | ||
| /// ```rust |
There was a problem hiding this comment.
What I do in doctests and comments is give a space between each of them like:
Suggested change
| /// # Error | |
| /// This function never errors. | |
| /// # Example | |
| /// ```rust | |
| /// # Error | |
| /// | |
| /// This function never errors. | |
| /// | |
| /// # Example | |
| /// | |
| /// ```rust |
alamb
approved these changes
Nov 18, 2020
There was a problem hiding this comment.
This looks good to me, FWIW.
It seems to me ArrayRef is only valuable if the code needs a very cheap way to copy the entire input (e.g. to another record batch). Given that is not the case with is_not_null I think removing the Arc wrapper makes the API more general. 👍
|
CI failure looks unrelated .https://github.com/apache/arrow/pull/8701/checks?check_run_id=1416672486 I am restarting it |
GeorgeAp
pushed a commit
to sirensolutions/arrow
that referenced
this pull request
Jun 7, 2021
…ignature. The change in signature was motivated while writing the example: there is no reason to wrap a concrete array on an `Arc` just to be able to pass it to the kernel. All kernels require an immutable reference to anything that implements `Array`, and thus asking for `&Arc<dyn Array>` seems to be equivalent / worse as asking for `&Vec<>` instead of `&[]` that clippy complaints about. Apart from that, this adds the examples. Closes apache#8701 from jorgecarleitao/is_null_array Authored-by: Jorge C. Leitao <jorgecarleitao@gmail.com> Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The change in signature was motivated while writing the example: there is no reason to wrap a concrete array on an
Arcjust to be able to pass it to the kernel. All kernels require an immutable reference to anything that implementsArray, and thus asking for&Arc<dyn Array>seems to be equivalent / worse as asking for&Vec<>instead of&[]that clippy complaints about.Apart from that, this adds the examples.