Fokko commented on code in PR #1426:
URL: https://github.com/apache/iceberg-python/pull/1426#discussion_r1890305553
##########
pyiceberg/table/name_mapping.py:
##########
@@ -49,9 +49,10 @@ def convert_null_to_empty_List(cls, v: Any) -> Any:
@model_serializer
def ser_model(self) -> Dict[str, Any]:
"""Set custom serializer to leave out the field when it is empty."""
+ field_id = {"field-id": self.field_id} if self.field_id is not None
else {}
fields = {"fields": self.fields} if len(self.fields) > 0 else {}
return {
- "field-id": self.field_id,
+ **field_id,
"names": self.names,
**fields,
}
Review Comment:
More a style thing, I think it is a bit awkward to merge all the dicts, how
about:
```python
serialized = {
"names": self.names
}
if self.field_id is not None:
serialized['field-id'] = self.field_id
if len(self.fields) > 0:
serialized['fields'] = fields
return serialized
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]