alamb commented on code in PR #10123:
URL: https://github.com/apache/arrow-rs/pull/10123#discussion_r3398448931
##########
arrow-array/src/array/map_array.rs:
##########
@@ -359,6 +363,91 @@ impl MapArray {
Ok(MapArray::from(map_data))
}
+
+ /// Helper to create [`MapArray`] from [`Vec`]s of entries so the code
will look clean and straightforward
Review Comment:
This is great
Is there any chance you can add a reference to this function from the
MapArray docs itself
```rust
/// See [`MapBuilder`](crate::builder::MapBuilder) for how to construct a
[`MapArray`]
#[derive(Clone)]
pub struct MapArray {
```
To something like
```rust
/// # See also
/// * [`MapBuilder`](crate::builder::MapBuilder) for how to construct a
[`MapArray`]
/// * [`Self::from_vec_of_maps`] for ergonomically creating maps for testing
#[derive(Clone)]
pub struct MapArray {
##########
arrow-array/src/array/map_array.rs:
##########
@@ -359,6 +363,91 @@ impl MapArray {
Ok(MapArray::from(map_data))
}
+
+ /// Helper to create [`MapArray`] from [`Vec`]s of entries so the code
will look clean and straightforward
+ ///
+ /// the input is: `Vec<Option<Map>>` where each `Map` is `Vec<(Key,
Option<Value>)>`
+ ///
+ /// Useful for tests, this should not be used for performance sensitive
operations
+ ///
+ /// ```
+ /// use std::collections::HashMap;
+ /// # use arrow_array::{MapArray, Int32Array, StringArray};
+ ///
+ /// let map = vec![
+ /// // {}
+ /// Some(vec![]),
+ /// // null
+ /// None,
+ /// // { "a": 1, "b": null, "cd": 4 }
+ /// Some(vec![
+ /// ("a", Some(1)),
+ /// ("b", None),
+ /// ("cd", Some(4)),
+ /// ]),
+ /// // { "e": 0 }
+ /// Some(vec![("e", Some(0))]),
+ /// ];
+ /// let ordered = true;
+ ///
+ /// // created map: [{}, null, {"a": 1, "b": null, "cd": 4}, {"e": 0}]
+ /// let map_array = MapArray::from_vec_of_maps::<StringArray, Int32Array,
_, _>(map, ordered);
+ /// // Or you could fill the last 2 generics manually for the key array
item and value array item
+ /// // let map_array = MapArray::from_vec_of_maps::<StringArray,
Int32Array, &str, i32>(map, ordered);
+ ///```
+ #[allow(clippy::type_complexity)]
Review Comment:
I agree it is ok to ignore the clippy warning here given the nice
documentation and example
##########
arrow-flight/src/encode.rs:
##########
@@ -1505,34 +1505,24 @@ mod tests {
let expected_schema = Arc::new(expected_schema);
- // Builder without dictionary fields
- let mut builder = MapBuilder::new(
- None,
- GenericStringBuilder::<i32>::new(),
- GenericStringBuilder::<i32>::new(),
+ // array without dictionary fields
Review Comment:
that is certainly much nicer and less painful on the eyes
--
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]
