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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org