Skip to content

Conversation

@friendlymatthew
Copy link
Contributor

Rationale for this change

This PR adds VariantPath::is_empty which checks whether VariantPath contains an empty list of VariantPathElements.

I found this API to be helpful when validating a VariantPath, since a VariantPath is only meaningful if it holds a nonempty list of VariantPathElements

@github-actions github-actions bot added the parquet-variant parquet-variant* crates label Nov 5, 2025
@friendlymatthew friendlymatthew force-pushed the friendlymatthew/variant-path-is-empty branch from bc00c60 to 8e2a0ca Compare November 5, 2025 21:38
@friendlymatthew
Copy link
Contributor Author

cc @alamb @scovich @klion26

@friendlymatthew friendlymatthew force-pushed the friendlymatthew/variant-path-is-empty branch from 8e2a0ca to ee62996 Compare November 5, 2025 21:41
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

makes sense to me -- thanks @friendlymatthew

Copy link
Contributor

@scovich scovich left a comment

Choose a reason for hiding this comment

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

Seems fine, but shouldn't the existing impl Deref (L130 below) transparently provide access to slice.is_empty? (c.f. playground)

Copy link
Member

@klion26 klion26 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the work

@alamb alamb merged commit e96e609 into apache:main Nov 6, 2025
17 checks passed
@alamb
Copy link
Contributor

alamb commented Nov 6, 2025

Seems fine, but shouldn't the existing impl Deref (L130 below) transparently provide access to slice.is_empty? (c.f. playground)

I think having an explicit method is probably still helpful even if it is strictly unecessary

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.

4 participants