Jefffrey commented on code in PR #10840:
URL: https://github.com/apache/arrow-rs/pull/10840#discussion_r3873028281
##########
arrow-schema/src/datatype_display.rs:
##########
@@ -474,24 +490,34 @@ mod tests {
#[test]
fn test_display_run_end_encoded() {
+ // Compact form: default field names "run_ends" and "values"
let run_ends_field = Arc::new(Field::new("run_ends", DataType::UInt32,
false));
let values_field = Arc::new(Field::new("values", DataType::Int32,
true));
- let ree_data_type = DataType::RunEndEncoded(run_ends_field.clone(),
values_field.clone());
- let ree_data_type_string = ree_data_type.to_string();
- let expected_string = "RunEndEncoded(\"run_ends\": non-null UInt32,
\"values\": Int32)";
- assert_eq!(ree_data_type_string, expected_string);
+ let ree = DataType::RunEndEncoded(run_ends_field.clone(),
values_field.clone());
+ assert_eq!(ree.to_string(), "RunEndEncoded(UInt32, Int32)");
+
+ // Compact form: non-null values
+ let run_ends_field = Arc::new(Field::new("run_ends", DataType::Int32,
false));
+ let values_field_str = Arc::new(Field::new("values", DataType::Utf8,
false));
+ let ree2 = DataType::RunEndEncoded(run_ends_field, values_field_str);
+ assert_eq!(ree2.to_string(), "RunEndEncoded(Int32, non-null Utf8)");
+
+ // Verbose form: non-default field name on values
+ let run_ends_field = Arc::new(Field::new("run_ends", DataType::Int32,
false));
+ let named_values = Arc::new(Field::new("named_values", DataType::Utf8,
false));
+ let ree3 = DataType::RunEndEncoded(run_ends_field, named_values);
+ assert_eq!(
+ ree3.to_string(),
+ "RunEndEncoded('run_ends': Int32, 'named_values': non-null Utf8)"
+ );
- // Test with metadata
Review Comment:
we probably should keep the metadata formatting too
##########
arrow-schema/src/datatype_display.rs:
##########
@@ -173,11 +173,27 @@ impl Display for DataType {
Ok(())
}
Self::RunEndEncoded(run_ends_field, values_field) => {
+ let default_names =
+ run_ends_field.name() == "run_ends" && values_field.name()
== "values";
Review Comment:
maybe in a followup PR we can create constants for these like so
https://github.com/apache/arrow-rs/blob/4ae884085e52305dab5515f6c21f28e8b537bb06/arrow-schema/src/field.rs#L151-L164
##########
arrow-schema/src/datatype_display.rs:
##########
@@ -474,24 +490,34 @@ mod tests {
#[test]
fn test_display_run_end_encoded() {
+ // Compact form: default field names "run_ends" and "values"
let run_ends_field = Arc::new(Field::new("run_ends", DataType::UInt32,
false));
let values_field = Arc::new(Field::new("values", DataType::Int32,
true));
- let ree_data_type = DataType::RunEndEncoded(run_ends_field.clone(),
values_field.clone());
- let ree_data_type_string = ree_data_type.to_string();
- let expected_string = "RunEndEncoded(\"run_ends\": non-null UInt32,
\"values\": Int32)";
- assert_eq!(ree_data_type_string, expected_string);
+ let ree = DataType::RunEndEncoded(run_ends_field.clone(),
values_field.clone());
+ assert_eq!(ree.to_string(), "RunEndEncoded(UInt32, Int32)");
+
+ // Compact form: non-null values
+ let run_ends_field = Arc::new(Field::new("run_ends", DataType::Int32,
false));
+ let values_field_str = Arc::new(Field::new("values", DataType::Utf8,
false));
+ let ree2 = DataType::RunEndEncoded(run_ends_field, values_field_str);
+ assert_eq!(ree2.to_string(), "RunEndEncoded(Int32, non-null Utf8)");
+
+ // Verbose form: non-default field name on values
+ let run_ends_field = Arc::new(Field::new("run_ends", DataType::Int32,
false));
+ let named_values = Arc::new(Field::new("named_values", DataType::Utf8,
false));
+ let ree3 = DataType::RunEndEncoded(run_ends_field, named_values);
+ assert_eq!(
+ ree3.to_string(),
+ "RunEndEncoded('run_ends': Int32, 'named_values': non-null Utf8)"
Review Comment:
map/structs/unions seem to favour double quotes; it seems only lists use
single quotes, but thats because its a value and not a key 🤔
https://github.com/apache/arrow-rs/blob/4ae884085e52305dab5515f6c21f28e8b537bb06/arrow-schema/src/datatype_display.rs#L468
https://github.com/apache/arrow-rs/blob/4ae884085e52305dab5515f6c21f28e8b537bb06/arrow-schema/src/datatype_display.rs#L410
https://github.com/apache/arrow-rs/blob/4ae884085e52305dab5515f6c21f28e8b537bb06/arrow-schema/src/datatype_display.rs#L329
##########
arrow-schema/src/datatype_parse.rs:
##########
@@ -1217,20 +1249,44 @@ mod test {
true,
),
DataType::RunEndEncoded(
- Arc::new(Field::new("run_ends", DataType::UInt32, false)),
+ Arc::new(Field::new("run_ends", DataType::UInt32, true)),
Arc::new(Field::new("values", DataType::Int32, true)),
),
DataType::RunEndEncoded(
Arc::new(Field::new(
- "nested_run_end_encoded",
+ "run_ends",
DataType::RunEndEncoded(
- Arc::new(Field::new("run_ends", DataType::UInt32,
false)),
+ Arc::new(Field::new("run_ends", DataType::UInt32,
true)),
Review Comment:
why are these nullabilities changed
--
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]
