IO: Consolidate PyArrow logic into io/pyarrow.py before decomposition #3812

Activity

1. qzyu999 commented on Aug 19, 2026

   

   qzyu999

   on Aug 19, 2026

   ContributorAuthor

   More actions

   Notes on inspect.py and transforms.py

   Adding some analysis on the two "TBD" items to guide discussion.

   table/inspect.py

   Every method in InspectTable follows the same pattern: iterate over metadata objects, build a list of Python dicts, define a pa.schema(...), and call pa.Table.from_pylist(data, schema=schema).

   Absorption would mean moving the schema definitions and from_pylist calls into helper functions in io/pyarrow.py:

   # io/pyarrow.py
   def _build_snapshots_table(snapshots: list[dict[str, Any]]) -> pa.Table:
       schema = pa.schema([
           pa.field("committed_at", pa.timestamp(unit="ms"), nullable=False),
           pa.field("snapshot_id", pa.int64(), nullable=False),
           ...
       ])
       return pa.Table.from_pylist(snapshots, schema=schema)

   Then inspect.py would only gather dicts and delegate:

   # table/inspect.py
   from pyiceberg.io.pyarrow import _build_snapshots_table
   
   def snapshots(self) -> "pa.Table":
       rows = [{"committed_at": ..., ...} for s in self.tbl.metadata.snapshots]
       return _build_snapshots_table(rows)

   My read: This is technically doable but adds ~10 trivial wrapper functions whose only job is pa.Table.from_pylist(data, schema=X). There's no compute here that benefits from engine substitution. A DataFusion engine would not build metadata inspection tables any differently. This is output formatting, not a substitution target

   Leaning toward leave, but open to feedback.

   transforms.py

   Each transform subclass defines a pyarrow_transform() method returning a Callable[[pa.Array], pa.Array]. The shared _pyiceberg_transform_wrapper dispatches on pa.Array vs pa.ChunkedArray.

   Absorption would mean moving the implementations out of the classes and into a dispatch function in io/pyarrow.py:

   # io/pyarrow.py
   def _get_pyarrow_transform(transform: Transform, source: IcebergType) -> Callable[[pa.Array], pa.Array]:
       if isinstance(transform, BucketTransform):
           return _pyiceberg_transform_wrapper(core.bucket, transform._num_buckets)
       elif isinstance(transform, YearTransform):
           return _pyiceberg_transform_wrapper(core.year, expected_type=pa.int32())
       ...

   My read: This fights the existing OO design. The pyarrow_transform() methods are polymorphic for a reason: each transform subclass owns its own implementation. Pulling them into a central isinstance dispatch:

   • Mirrors the class hierarchy in a fragile way (adding a new transform requires editing two places)
   • Doesn't improve substitutability. If DataFusion came in, you'd add a parallel datafusion_transform() method or a generic transform(engine) dispatch alongside the existing methods, not replace them.

   The methods are already scoped by name (pyarrow_transform) to signal they're the PyArrow-specific path. They are the extension point.

   Leaning toward leave.

   Summary

   File	Absorption approach	Recommendation
   inspect.py	Move schema defs + from_pylist into io/pyarrow.py helpers	Leave. No compute to substitute, just output formatting
   transforms.py	Central dispatch function replacing polymorphic methods	Leave. Breaks OO design, methods already serve as the named extension point

   Happy to reconsider if there's a use case I'm missing.
2. qzyu999

   added 2 commits that reference this issue on Sep 9, 2026

   IO: Absorb upsert PyArrow logic into io/pyarrow.py

   ...

   This issue will close once commit 9ec5506 is merged into the 'main' branch.

   9ec5506

   IO: Absorb upsert PyArrow logic into io/pyarrow.py

   ...

   This issue will close once commit 0a9ddaf is merged into the 'main' branch.

   0a9ddaf