kevinjqliu commented on code in PR #1426:
URL: https://github.com/apache/iceberg-python/pull/1426#discussion_r1888945215
##########
pyiceberg/table/name_mapping.py:
##########
@@ -232,7 +234,9 @@ def mapping(self, nm: NameMapping, field_results:
List[MappedField]) -> List[Map
def fields(self, struct: List[MappedField], field_results:
List[MappedField]) -> List[MappedField]:
reassignments: Dict[str, int] = {
- update.name: update.field_id for f in field_results if (update :=
self._updates.get(f.field_id))
+ update.name: update.field_id
+ for f in field_results
+ if f.field_id is not None and (update :=
self._updates.get(f.field_id))
Review Comment:
what is the rationale behind this change? Should we look at all the other
places where `.field_id` is used?
##########
pyiceberg/table/name_mapping.py:
##########
@@ -333,8 +334,8 @@ def struct(self, struct: StructType, struct_partner:
Optional[MappedField], fiel
return StructType(*field_results)
def field(self, field: NestedField, field_partner: Optional[MappedField],
field_result: IcebergType) -> IcebergType:
- if field_partner is None:
- raise ValueError(f"Field missing from NameMapping:
{'.'.join(self.current_path)}")
+ if field_partner is None or field_partner.field_id is None:
Review Comment:
I see `field_id` is a required field in `NestedField`. Without
`field_partner.field_id is None`, type checker errors
--
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]
