feat: impl display for DataType::List
#7051
Conversation
tustvold
added
api-change
| DataType::List(Arc::new(Field::new_list_field(DataType::UInt64, false))); | ||
| let list_data_type_string = list_data_type.to_string(); | ||
| let expected_string = "List(UInt64)"; | ||
| assert_eq!(list_data_type_string, expected_string); |
There was a problem hiding this comment.
Perhaps we could get a test of a nested list as well
|
Thanks @tustvold , I have made the changes as you suggested. Could you please help review it again? |
arrow-schema/src/datatype.rs
Outdated
| @@ -458,7 +458,16 @@ pub enum UnionMode { | |||
|
|
|||
| impl fmt::Display for DataType { | |||
| fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | |||
| write!(f, "{self:?}") | |||
| match self { | |||
| DataType::List(field) => { | |||
There was a problem hiding this comment.
We also need to print the field name here if it isn't the default of item
There was a problem hiding this comment.
Right -- so if field.name() != "item" perhaps it could look like
List(Int8;N, field='foo')
Likewise, if there is metadata I think it should also be displayed like
List(Int8;N, field='foo', metadata)
|
The Arrow docstring on datatype says the Display should be also reversible - look at the code of datatype_parse it doesn't seem like lists were parseable even before, but maybe it'd be good to add as part of this PR given the new format should be easier to parse? :) |
arrow-schema/src/datatype.rs
Outdated
| @@ -458,7 +458,25 @@ pub enum UnionMode { | |||
|
|
|||
| impl fmt::Display for DataType { | |||
| fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | |||
| write!(f, "{self:?}") | |||
| match self { | |||
| DataType::List(field) => { | |||
There was a problem hiding this comment.
maybe add the same for LargeList and FixedSizeList?
There was a problem hiding this comment.
Thanks @Blizzara for your advice, that's good idea!
|
Thanks @irenjj for working on this. I agree with @Blizzara. The display result should be reversible. I prefer to format the DataType::List(field) => {
write!(f, "List({})", field.data_type())
}The result would be something like arrow-rs/arrow-schema/src/datatype_parse.rs Line 604 in 7905545 |
Which issue does this PR close?
Closes #7048
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?