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

Reply via email to