BlakeOrth commented on issue #9113:
URL: https://github.com/apache/arrow-rs/issues/9113#issuecomment-3731268291
> I realize, looking now, that it's all rather indirect. But in theory, you
could so something that mimics the append_json function
Yes! After opening this issue and subsequently looking at the `parquet-json`
code I more or less landed on this as a solution (feeling a bit dumb that I
didn't realize this is how it was supposed to used). I more or less landed on
this exact solution you've shown here (even down to the method names and
signatures) with the exception that I didn't realize that I could simply use my
`VariantArrayBuilder` instead of using `VariantBuilder` and `append_value`.
> Back when I first defined VariantBuilderExt trait, I wondered if I should
have called it VariantBuilder and renamed the concrete type to something like
SingleVariantBuilder... maybe it's time to reconsider that idea?
Coming at this with fresh eyes I'm not sure the different naming (a truly
difficult computer science problem) would have helped immediately point me in
the right direction. The fact that
`VariantArrayBuilder::append_value(Into<Variant>)` exists made me feel like
that was the "correct" way to build an array of `Variant`. I feel like the
names and docs around `new_object()` and `new_list()` don't communicate that
you're adding a new element to the Array for the object/list that's going to be
built. Even re-reading the docs after this conversation I'm still not 100% sure
that's what's going to happen?
For instance, this seems very clear:
```rust
let mut builder = VariantArrayBuilder::new();
for i in 0..3 {
builder.append_value(i)
}
// result:
// +--------------+
// | VariantArray |
// +--------------+
// | 1 |
// | 2 |
// | 3 |
// +--------------+
```
But with `new_object()` it's less clear:
```rust
let mut builder = VariantArrayBuilder::new();
for i in 0..3 {
let obj = builder.new_object();
obj.append_value(i);
obj.finish();
}
// I believe I get this
// +--------------+
// | VariantArray |
// +--------------+
// | {1} |
// | {2} |
// | {3} |
// +--------------+
// But the lack of 'append' makes me feel I might get this
// +--------------+
// | VariantArray |
// +--------------+
// | {1, 2, 3} |
// +--------------+
```
I think after having this conversation the code that's there makes more
sense, and I do understand why the names that have been chosen are there. I
also just reviewed the example for `VariantArrayBuilder` and see that it does a
good job of clarifying this, so I'll blame my lack of careful reading for that
initial misunderstanding. I would also be willing to contribute to the
documentation with an additional example and maybe some clarity on the builder
methods about the intended usage (such as pointing users who land on
`VariantBuilder` to prefer `VariantArrayBuilder` etc.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
