Address review: update SortField.__str__ and drop underscore keys from serializers

Author: moomindani

pyiceberg/partitioning.py

@@ -134,10 +134,8 @@ def _serialize_source_ids(self, handler: Any) -> Any:
         # multi-argument transforms write only source-ids
         if self.source_ids is not None and len(self.source_ids) > 1:
             serialized.pop("source-id", None)
-            serialized.pop("source_id", None)
         else:
             serialized.pop("source-ids", None)
-            serialized.pop("source_ids", None)
         return serialized
 
     def __str__(self) -> str:

pyiceberg/table/sorting.py

@@ -125,10 +125,8 @@ def _serialize_source_ids(self, handler: Any) -> Any:
         # multi-argument transforms write only source-ids
         if self.source_ids is not None and len(self.source_ids) > 1:
             serialized.pop("source-id", None)
-            serialized.pop("source_id", None)
         else:
             serialized.pop("source-ids", None)
-            serialized.pop("source_ids", None)
         return serialized
 
     source_id: int = Field(alias="source-id")
@@ -147,8 +145,11 @@ def __str__(self) -> str:
         if isinstance(self.transform, IdentityTransform):
             # In the case of an identity transform, we can omit the transform
             return f"{self.source_id} {self.direction} {self.null_order}"
+        if self.source_ids is not None and len(self.source_ids) > 1:
+            sources = ", ".join(str(s) for s in self.source_ids)
         else:
-            return f"{self.transform}({self.source_id}) {self.direction} {self.null_order}"
+            sources = str(self.source_id)
+        return f"{self.transform}({sources}) {self.direction} {self.null_order}"
 
 
 INITIAL_SORT_ORDER_ID = 1

tests/table/test_partitioning.py

@@ -292,6 +292,8 @@ def test_deserialize_partition_field_multi_arg() -> None:
     assert "source-id" not in serialized
     assert serialized["transform"] == "bucket[4]"
 
+    assert str(field) == "1000: multi_bucket: bucket[4](1, 2)"
+
 
 def test_serialize_partition_field_single_source_id_only() -> None:
     import json as json_lib

tests/table/test_sorting.py

@@ -193,6 +193,8 @@ def test_deserialize_sort_field_multi_arg() -> None:
     assert "source-id" not in serialized
     assert serialized["transform"] == "bucket[4]"
 
+    assert str(field) == "bucket[4](19, 20) ASC NULLS FIRST"
+
 
 def test_deserialize_sort_field_multi_arg_requires_transform() -> None:
     payload = '{"source-ids":[19,20],"direction":"asc","null-order":"nulls-first"}'