rambleraptor commented on code in PR #3630:
URL: https://github.com/apache/iceberg-python/pull/3630#discussion_r3825819940
##########
pyiceberg/partitioning.py:
##########
@@ -115,13 +117,34 @@ def map_source_ids_onto_source_id(cls, data: Any) -> Any:
if len(source_ids) == 0:
raise ValueError("Empty source-ids is not allowed")
if len(source_ids) > 1:
- raise ValueError("Multi argument transforms are not
yet supported")
+ if data.get("transform") is None:
Review Comment:
This block of code doesn't get run if `source-id` and `source-ids` are both
set.
In general, this set of code is way too deeply-nested and it's hard to
understand.
##########
pyiceberg/partitioning.py:
##########
@@ -77,6 +78,7 @@ class PartitionField(IcebergBaseModel):
"""
source_id: int = Field(alias="source-id")
+ source_ids: list[int] | None = Field(alias="source-ids", default=None,
repr=False)
Review Comment:
Is it possible for us to add a "source-id" getter to this method? We've got
places in the code that work off data["source-id"] and need to be updated for
source-ids.
If we can consolidate where source-id is accessed, it makes it a lot easier
to handle the source-id vs. source_ids thing.
Unfortunate downside of our dependency on Pydantic.
--
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]
