tustvold commented on code in PR #4706:
URL: https://github.com/apache/arrow-rs/pull/4706#discussion_r1296100137
##########
parquet/src/schema/types.rs:
##########
@@ -560,28 +559,30 @@ impl<'a> GroupTypeBuilder<'a> {
}
/// Sets [`ConvertedType`](crate::basic::ConvertedType) for this field and
returns itself.
- pub fn with_converted_type(mut self, converted_type: ConvertedType) ->
Self {
- self.converted_type = converted_type;
- self
+ pub fn with_converted_type(self, converted_type: ConvertedType) -> Self {
+ Self {
+ converted_type,
+ ..self
+ }
}
/// Sets [`LogicalType`](crate::basic::LogicalType) for this field and
returns itself.
- pub fn with_logical_type(mut self, logical_type: Option<LogicalType>) ->
Self {
- self.logical_type = logical_type;
- self
+ pub fn with_logical_type(self, logical_type: Option<LogicalType>) -> Self {
+ Self {
+ logical_type,
+ ..self
+ }
}
/// Sets a list of fields that should be child nodes of this field.
/// Returns updated self.
- pub fn with_fields(mut self, fields: &mut Vec<TypePtr>) -> Self {
- self.fields.append(fields);
- self
+ pub fn with_fields(self, fields: Vec<TypePtr>) -> Self {
+ Self { fields, ..self }
}
/// Sets optional field id and returns itself.
- pub fn with_id(mut self, id: i32) -> Self {
- self.id = Some(id);
- self
+ pub fn with_id(self, id: Option<i32>) -> Self {
Review Comment:
This makes with_id consistent with the other APIs such as
`with_logical_type` and `with_converted_type` (where `ConvertedType has
implicit nullability as a result of `ConvertedType::NONE`). It also reflects
the fact this setting is most often optional
--
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]
