[Variant] Improve documentation and includes for casts #8532
+22
−19
Conversation
This file contains hidden or 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
github-actions
bot
added
parquet
alamb
commented
Oct 1, 2025
| /// # use arrow::array::{Array, ArrayRef, Int64Array}; | ||
| /// # use parquet_variant::Variant; | ||
| /// # use parquet_variant_compute::cast_to_variant::cast_to_variant; | ||
| /// # use parquet_variant_compute::cast::cast_to_variant; |
There was a problem hiding this comment.
I renamed this module cast so that it doesn't have the same name as the function cast_to_variant
There was a problem hiding this comment.
but then it doesn't match the convention of shred_variant and unshred_variant 🤔
| //! - [`variant_to_json`]: Function to convert a `VariantArray` to Arrays of JSON strings. | ||
| //! - [`cast_to_variant`]: Cast Arrow arrays to `VariantArray`. | ||
| //! - [`variant_get`]: Convert `VariantArray` (or an inner path) to ArrowArrays type | ||
| //! - [`shred_variant`]: Shred a `VariantArray` |
There was a problem hiding this comment.
added mention of shred and unshred
| mod arrow_to_variant; | ||
| pub mod cast_to_variant; | ||
| mod cast; |
There was a problem hiding this comment.
let's keep it consistent and not have pub modules, and instead pub use the parts of the API explicitly
alamb
commented
Oct 1, 2025
| /// # use arrow::array::{Array, ArrayRef, Int64Array}; | ||
| /// # use parquet_variant::Variant; | ||
| /// # use parquet_variant_compute::cast_to_variant::cast_to_variant; | ||
| /// # use parquet_variant_compute::cast::cast_to_variant; |
There was a problem hiding this comment.
but then it doesn't match the convention of shred_variant and unshred_variant 🤔
pull bot
pushed a commit
to yeya24/arrow-rs
that referenced
this pull request
Oct 4, 2025
) # Which issue does this PR close? We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. - Closes apache#8531 - Closes apache#8532 # Rationale for this change Basically, I was confused about the casting / conversion functions available so I want to improve the documentation # What changes are included in this PR? 1. Add documentation, improve other docs 2. Remove `pub` crates in favor of exporting only the functions/ structs needed # Are these changes tested? Yes by CI If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? # Are there any user-facing 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 call them out. --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Ryan Johnson <[email protected]>
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.
Which issue does this PR close?
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax.
Rationale for this change
Basically, I was confused about the casting / conversion functions available so I want to improve the documentation
What changes are included in this PR?
cast_to_variantmodule tocastso there isn't a function with the same nameAre these changes tested?
We typically require tests for all PRs in order to:
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
Are there any user-facing 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 call them out.