kevinjqliu commented on code in PR #1768:
URL: https://github.com/apache/iceberg-python/pull/1768#discussion_r2051557682
##########
tests/avro/test_file.py:
##########
@@ -335,22 +317,49 @@ def test_all_primitive_types(is_required: bool) -> None:
)
class AllPrimitivesRecord(Record):
- field_fixed: bytes
- field_decimal: Decimal
- field_bool: bool
- field_int: int
- field_long: int
- field_float: float
- field_double: float
- field_date: date
- field_time: time
- field_timestamp: datetime
- field_timestamptz: datetime
- field_string: str
- field_uuid: UUID
-
- def __init__(self, *data: Any, **named_data: Any) -> None:
- super().__init__(*data, **{"struct":
all_primitives_schema.as_struct(), **named_data})
+ @property
+ def field_fixed(self) -> bytes:
+ return self._data[0]
+
+ @property
+ def field_decimal(self) -> Decimal:
+ return self._data[1]
+
+ @property
+ def field_bool(self) -> bool:
+ return self._data[2]
+
+ @property
+ def field_int(self) -> int:
+ return self._data[3]
+
+ @property
+ def field_long(self) -> int:
+ return self._data[4]
+
+ @property
+ def field_float(self) -> float:
+ return self._data[5]
+
+ @property
+ def field_double(self) -> float:
+ return self._data[6]
+
Review Comment:
this is missing
```
field_date: date
field_time: time
```
is that intentional?
##########
pyiceberg/avro/reader.py:
##########
@@ -312,7 +312,15 @@ def skip(self, decoder: BinaryDecoder) -> None:
class StructReader(Reader):
- __slots__ = ("field_readers", "create_struct", "struct",
"_create_with_keyword", "_field_reader_functions", "_hash")
+ __slots__ = (
+ "field_readers",
+ "create_struct",
+ "struct",
+ "_create_with_keyword",
Review Comment:
nit: looks like we got rid of `_create_with_keyword`
##########
tests/integration/test_partitioning_key.py:
##########
@@ -792,6 +792,5 @@ def test_partition_key(
snapshot.manifests(iceberg_table.io)[0].fetch_manifest_entry(iceberg_table.io)[0].data_file.file_path
)
# Special characters in partition value are sanitized when written to
the data file's partition field
- sanitized_record = Record(**{make_compatible_name(k): v for k, v in
vars(expected_partition_record).items()})
- assert spark_partition_for_justification == sanitized_record
+ assert spark_partition_for_justification == expected_partition_record
Review Comment:
love this!!
##########
pyiceberg/typedef.py:
##########
@@ -171,51 +171,36 @@ class IcebergRootModel(RootModel[T], Generic[T]):
model_config = ConfigDict(frozen=True)
-@lru_cache
-def _get_struct_fields(struct_type: StructType) -> Tuple[str, ...]:
- return tuple(field.name for field in struct_type.fields)
-
-
class Record(StructProtocol):
- __slots__ = ("_position_to_field_name",)
- _position_to_field_name: Tuple[str, ...]
-
- def __init__(self, *data: Any, struct: Optional[StructType] = None,
**named_data: Any) -> None:
- if struct is not None:
- self._position_to_field_name = _get_struct_fields(struct)
- elif named_data:
- # Order of named_data is preserved (PEP 468) so this can be used
to generate the position dict
- self._position_to_field_name = tuple(named_data.keys())
- else:
- self._position_to_field_name = tuple(f"field{idx + 1}" for idx in
range(len(data)))
+ __slots__ = ("_data",)
+ _data: List[Any]
- for idx, d in enumerate(data):
- self[idx] = d
+ @classmethod
+ def _bind(cls, struct: StructType, **arguments: Any) -> Self:
+ return cls(*[arguments[field.name] if field.name in arguments else
field.initial_default for field in struct.fields])
Review Comment:
ah ok `initial_default` is `None` if not set 👍
--
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
