alamb commented on code in PR #1703:
URL: https://github.com/apache/arrow-rs/pull/1703#discussion_r873635265
##########
arrow/src/datatypes/datatype.rs:
##########
@@ -115,7 +115,7 @@ pub enum DataType {
/// A nested datatype that contains a number of sub-fields.
Struct(Vec<Field>),
/// A nested datatype that can represent slots of differing types.
Review Comment:
```suggestion
/// A nested datatype that can represent slots of differing types.
Components:
///
/// 1. [`Field`] for each possible child type the Union can hold
/// 2. The corresponding `type_id` used to identify which Field
/// 3. The type of union (Sparse or Dense)
```
##########
arrow/src/datatypes/datatype.rs:
##########
@@ -516,24 +516,15 @@ impl DataType {
.as_array()
.unwrap()
.iter()
- .map(|t| t.as_i64().unwrap())
+ .map(|t| t.as_i64().unwrap() as i8)
Review Comment:
maybe we could call `t.as_i8()`?
##########
integration-testing/src/lib.rs:
##########
@@ -632,39 +632,13 @@ fn array_from_json(
let array = MapArray::from(array_data);
Ok(Arc::new(array))
}
- DataType::Union(fields, _) => {
Review Comment:
👍 This certainly makes it easier to understand
##########
arrow/src/datatypes/mod.rs:
##########
@@ -435,19 +435,10 @@ mod tests {
"my_union",
DataType::Union(
vec![
- Field::new("f1", DataType::Int32, true).with_metadata(Some(
Review Comment:
why was the `metadata` removed from this test? Is it simply no longer
necessary?
--
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]
