[Variant] Consolidate examples for json writing #7782
Conversation
| /// | ||
| /// | ||
| /// ```rust | ||
| /// # use parquet_variant::{Variant, variant_to_json}; |
There was a problem hiding this comment.
I removed the redundant examples for Variant::Int32 and Variant::String, here and below, as I didn't think they were substantially different.
| /// let mut buffer = Vec::new(); | ||
| /// variant_to_json(&mut buffer, &variant)?; | ||
| /// assert_eq!(String::from_utf8(buffer).unwrap(), "\"Hello, World!\""); | ||
| /// let mut builder = VariantBuilder::new(); |
There was a problem hiding this comment.
The nested object is a great example and I wanted to have that again on variant_to_json as well
|
@carpecodeum I wonder if you have some time to review this PR? |
parquet-variant/src/to_json.rs
Outdated
| /// # use parquet_variant::{Variant, variant_to_json}; | ||
| /// # use arrow_schema::ArrowError; | ||
| /// let variant = Variant::Int32(42); | ||
| /// let variant = Variant::String("Hello, World!"); |
There was a problem hiding this comment.
Hi, I just realized there's nothing stopping a user from creating a Variant::String that could've been a Variant::ShortString.
This isn't the end of the world, but using the String variant adds 4 unnecessary bytes to the header, which could accumulate in loops or large payloads.
There was a problem hiding this comment.
I agree. My understanding is that a smaller string for Variant::Stirng is still valid, just less efficient.
Maybe I should update the examples to use Variant::from which does the right thing
|
Thank you for the review @carpecodeum |
3 participants
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
I was going through the code and examples and I felt that there was some redundancy and that the example file was unlikely to be found as not many crates in this repo have examples
I would like to propose moving the examples as close to the actual code as possible to give it the best chance to be discovered.
What changes are included in this PR?
parquet-variant/examples/variant_to_json_examples.rsAre these changes tested?
The examples are covered by CI tests.
Are there any user-facing changes?
Different docs