Anton-Tarazi commented on code in PR #2410:
URL: https://github.com/apache/iceberg-python/pull/2410#discussion_r2374059339
##########
pyiceberg/partitioning.py:
##########
@@ -245,6 +246,37 @@ def partition_to_path(self, data: Record, schema: Schema)
-> str:
path = "/".join([field_str + "=" + value_str for field_str, value_str
in zip(field_strs, value_strs)])
return path
+ @staticmethod
+ def check_compatibility(partition_spec: PartitionSpec, schema: Schema,
allow_missing_fields: bool = False) -> None:
Review Comment:
same here
##########
pyiceberg/table/sorting.py:
##########
@@ -168,6 +171,18 @@ def __repr__(self) -> str:
fields = f"{', '.join(repr(column) for column in self.fields)}, " if
self.fields else ""
return f"SortOrder({fields}order_id={self.order_id})"
+ @staticmethod
+ def check_compatibility(sort_order: SortOrder, schema: Schema) -> None:
Review Comment:
It strikes me as a bit of an anti-pattern to have a static method whose
first argument is of the type of its class, could we make this a normal method?
Maybe renaming to `check_compatible` in that case would be clearer
--
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]
