[refactor] Make StructType and DataType::struct_type more general

[scovich]

Constructing new StructType instances has historically been more verbose than necessary because they require a Vec<StructField>. Update the various constructors to accept impl IntoIterator instead, and simplify call sites to take advantage.

[codecov]

Codecov Report

Attention: Patch coverage is 96.70330% with 3 lines in your changes missing coverage. Please review.

Project coverage is 77.68%. Comparing base (463bc7e) to head (d4d2551).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
kernel/src/schema.rs	84.21%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #385   +/-   ##
=======================================
  Coverage   77.67%   77.68%           
=======================================
  Files          49       49           
  Lines       10089    10084    -5     
  Branches    10089    10084    -5     
=======================================
- Hits         7837     7834    -3     
  Misses       1805     1805           
+ Partials      447      445    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zachschuermann]

awesome this is better ergonomics! one question but LGTM

[zachschuermann]

do we have the same From impl for the struct_type above?

[scovich]

No, because it seemed one level of indirection too many to have From<Iterator>.

I guess we could consider implementing FromIterator for StructType? But we'd still need either DataType::struct_type or another impl FromIterator... and at that point things are uncomfortably indirect?

let data_type: DataType = some_iter.map(...).collect();

Also, there's no corresponding TryFromIterator trait to replace StructType::try_new and DataType::try_struct_type.

[zachschuermann]

yea makes sense. seems fine to leave :)

[scovich]

Oh. My. Maybe we should impl FromIterator. This actually works:

impl FromIterator<StructField> for StructType {
    fn from_iter<T: IntoIterator<Item = StructField>>(iter: T) -> Self {
        Self {
            type_name: "struct".into(),
            fields: iter.into_iter().map(|f| (f.name.clone(), f)).collect(),
        }
    }
}

impl StructType {
    pub fn new(fields: impl IntoIterator<Item = StructField>) -> Self {
        fields.into_iter().collect()
    }

    pub fn try_new<E>(fields: impl IntoIterator<Item = Result<StructField, E>>) -> Result<Self, E> {
        fields.into_iter().try_collect()
    }

(Itertools::try_collect Just Works)

But this would only be to make new and try_new nicer -- I checked all the various call sites and almost none of them benefit from using collect directly.

[nicklan]

I agree that's nice, but it'll be a mess if we ever need another argument for creating a struct, so given only local benefit I'd err on the side of not implementing FromIterator

[nicklan]

nice, thanks!

[nicklan]

I agree that's nice, but it'll be a mess if we ever need another argument for creating a struct, so given only local benefit I'd err on the side of not implementing FromIterator