Skip to content

[Variant] add doc reference to VariantArrayBuilder#10152

Open
sdf-jkl wants to merge 2 commits into
apache:mainfrom
sdf-jkl:variant-builder-doc
Open

[Variant] add doc reference to VariantArrayBuilder#10152
sdf-jkl wants to merge 2 commits into
apache:mainfrom
sdf-jkl:variant-builder-doc

Conversation

@sdf-jkl

@sdf-jkl sdf-jkl commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Users might go for VariantBuilder to manually build VariantArrays.

Instead they should use VariantArrayBuilder.

What changes are included in this PR?

  • adds an example and a doc reference to VariantArrayBuilder for VariantBuilder

Are these changes tested?

n/a

Are there any user-facing changes?

doc change

@github-actions github-actions Bot added the parquet-variant parquet-variant* crates label Jun 16, 2026

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @sdf-jkl -- this is a nice improvement

I left a suggestion for making it even better -- let me know what you think

Comment thread parquet-variant/src/builder.rs Outdated
/// implements [`VariantBuilderExt`], so you append values and nested objects or
/// lists the same way:
///
/// ```ignore

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe this is marked as ignore as the VariantArrayBuilder lives in a different crate

It seems like it would be better to not ignore this doc so that it was tested and we ensure it is a complete example.

What do you think about leaving the text above

/// `VariantBuilder` builds a single, self-contained [`Variant`] value -- useful
/// for one-off values and unit tests. To build an array (column) of variants,
/// one per input row, use [`VariantArrayBuilder`] from the
/// `parquet-variant-compute` crate rather than a `VariantBuilder` per row. 

In this crate

And then adding the example and detail about VariantBuilderExt directly as an exampel on VariantArrayBuilder - that would likely make it easier to find

/// [`VariantArrayBuilder`] implements [`VariantBuilderExt`], so you append values and 
/// nested objects or lists the same way as when building a single Variant value with [`VariantBuilder`]:
///
/// ```
/// <real example ehre
/// ```

🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

964506d

That makes sense, thanks. I dropped the example because VariantArrayBuilder already has some.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parquet-variant parquet-variant* crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Variant] Improve ergonomics when using VariantBuilder

2 participants