Jefffrey commented on code in PR #10840:
URL: https://github.com/apache/arrow-rs/pull/10840#discussion_r3958004847
##########
arrow-schema/src/datatype_display.rs:
##########
@@ -173,11 +174,31 @@ impl Display for DataType {
Ok(())
}
Self::RunEndEncoded(run_ends_field, values_field) => {
+ let default_names = run_ends_field.name() ==
Field::REE_RUN_ENDS_FIELD_DEFAULT_NAME
+ && values_field.name() ==
Field::REE_VALUES_FIELD_DEFAULT_NAME;
write!(f, "RunEndEncoded(")?;
- let run_ends_str = format_field(run_ends_field);
- let values_str = format_field(values_field);
-
- write!(f, "{run_ends_str}, {values_str})")?;
+ let re_null = format_nullability(run_ends_field);
+ let v_null = format_nullability(values_field);
+ let v_meta = format_metadata(values_field.metadata());
+ if default_names {
+ write!(
+ f,
+ "{re_null}{}, {v_null}{}{})",
+ run_ends_field.data_type(),
+ values_field.data_type(),
+ v_meta,
+ )?;
+ } else {
+ write!(
+ f,
+ "\"{}\": {}, \"{}\": {v_null}{}{})",
Review Comment:
we should include the run ends nullability here too for consistency
##########
arrow-schema/src/datatype_display.rs:
##########
@@ -173,11 +174,31 @@ impl Display for DataType {
Ok(())
}
Self::RunEndEncoded(run_ends_field, values_field) => {
+ let default_names = run_ends_field.name() ==
Field::REE_RUN_ENDS_FIELD_DEFAULT_NAME
+ && values_field.name() ==
Field::REE_VALUES_FIELD_DEFAULT_NAME;
write!(f, "RunEndEncoded(")?;
- let run_ends_str = format_field(run_ends_field);
- let values_str = format_field(values_field);
-
- write!(f, "{run_ends_str}, {values_str})")?;
+ let re_null = format_nullability(run_ends_field);
+ let v_null = format_nullability(values_field);
+ let v_meta = format_metadata(values_field.metadata());
Review Comment:
i feel we should include both metadata of run ends & values, instead of only
values; likely we should fallback to the original format `"{run_ends_str},
{values_str})"` when we have any metadata at all
im imagining just two forms of display:
`RunEndEncoded(non-null UInt32, Int32)`
- simplified form
- only when no metadata in either, and field names are default
`RunEndEncoded(\"run_ends\": non-null UInt32, metadata: {\"key\":
\"value\"}, \"values\": Int32)`
- complex form (current form on main)
- when any metadata is present or field names are custom
--
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]
