Skip to content

Commit f86284f

Browse files
Tighten IncrementalAppendScan docstrings
Drop references to other Iceberg clients (Java, Spark) and re-shape the prose to match the style already used by `Table.scan` and `DataScan` (which the original PR #2235 also mirrored). - Trim `BaseScan` docstring to a single line, matching `TableScan`. - Reduce `IncrementalAppendScan` class docstring to the args block, mirroring `DataScan`'s shape. Roll the "from may be expired" note into the `from_snapshot_id_exclusive` arg description. - Match `from_snapshot_exclusive` / `to_snapshot_inclusive` chaining docstrings to PR #2235's wording ("this for method chaining"). - Simplify `is_parent_ancestor_of` and `plan_files` docstrings. - Drop the "matches iceberg-java" comment from the test parametrize. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e083a47 commit f86284f

3 files changed

Lines changed: 18 additions & 48 deletions

File tree

pyiceberg/table/__init__.py

Lines changed: 16 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1754,13 +1754,7 @@ def _parse_row_filter(expr: str | BooleanExpression) -> BooleanExpression:
17541754

17551755

17561756
class BaseScan(ABC):
1757-
"""A base class for all table scans.
1758-
1759-
Holds the snapshot-independent state (row filter, projected columns, case sensitivity,
1760-
options, limit) and shared chaining helpers. Concrete subclasses are :class:`TableScan`
1761-
(single-snapshot scans, e.g. :class:`DataScan`) and :class:`IncrementalAppendScan`
1762-
(a range of append-operation snapshots).
1763-
"""
1757+
"""A base class for all table scans."""
17641758

17651759
table_metadata: TableMetadata
17661760
io: FileIO
@@ -2230,29 +2224,17 @@ def count(self) -> int:
22302224

22312225

22322226
class IncrementalAppendScan(BaseScan):
2233-
"""An incremental scan that accumulates data appended between two snapshots.
2234-
2235-
The configured snapshot range corresponds to Spark's ``start-snapshot-id`` (exclusive)
2236-
and ``end-snapshot-id`` (inclusive) read options. An explicit ``from_snapshot_id_exclusive``
2237-
is required: defaulting to the table's oldest ancestor (as the Java API does) is rarely the
2238-
user's intent, and erroring is safer. ``to_snapshot_id_inclusive`` defaults to the table's
2239-
current snapshot when omitted.
2240-
2241-
Unsupported (relative to iceberg-java's ``IncrementalAppendScan``):
2242-
2243-
- Snapshot references / ``useBranch``: the configured snapshots must be passed as IDs.
2244-
- Counting rows (``count()``) and rolling time-travel via ``use_ref(...)``: these belong to
2245-
single-snapshot scans (:class:`TableScan` / :class:`DataScan`).
2227+
"""An incremental scan of a table's data that accumulates appended data between two snapshots.
22462228
22472229
Args:
22482230
row_filter:
22492231
A string or BooleanExpression that describes the
2250-
desired rows.
2232+
desired rows
22512233
selected_fields:
22522234
A tuple of strings representing the column names
22532235
to return in the output dataframe.
22542236
case_sensitive:
2255-
If True column matching is case sensitive.
2237+
If True column matching is case sensitive
22562238
options:
22572239
Additional Table properties as a dictionary of
22582240
string key value pairs to use for this scan.
@@ -2261,15 +2243,12 @@ class IncrementalAppendScan(BaseScan):
22612243
return in the scan result. If None, fetches all
22622244
matching rows.
22632245
from_snapshot_id_exclusive:
2264-
ID of the "from" snapshot, to start the incremental scan from, exclusively. When the
2265-
scan is ultimately planned, this must not be None. This snapshot does not need to be
2266-
present in the table metadata (it may have been expired), as long as it is the parent
2267-
of some ancestor of the "to" snapshot. Corresponds to Spark's ``start-snapshot-id``
2268-
read option.
2246+
Optional ID of the "from" snapshot, to start the incremental scan from, exclusively. When the scan is
2247+
ultimately planned, this must not be None. The snapshot does not need to be present in the table metadata
2248+
(it may have been expired), as long as it is the parent of some ancestor of the "to" snapshot.
22692249
to_snapshot_id_inclusive:
22702250
Optional ID of the "to" snapshot, to end the incremental scan at, inclusively.
2271-
Omitting it will default to the table's current snapshot. Corresponds to Spark's
2272-
``end-snapshot-id`` read option.
2251+
Omitting it will default to the table's current snapshot.
22732252
"""
22742253

22752254
from_snapshot_id_exclusive: int | None
@@ -2300,24 +2279,24 @@ def __init__(
23002279
self.to_snapshot_id_inclusive = to_snapshot_id_inclusive
23012280

23022281
def from_snapshot_exclusive(self: IAS, from_snapshot_id_exclusive: int | None) -> IAS:
2303-
"""Instruct this scan to look for changes starting from a particular snapshot (exclusive).
2282+
"""Instructs this scan to look for changes starting from a particular snapshot (exclusive).
23042283
23052284
Args:
2306-
from_snapshot_id_exclusive: the start snapshot ID (exclusive).
2285+
from_snapshot_id_exclusive: the start snapshot ID (exclusive)
23072286
23082287
Returns:
2309-
A copy of this scan with the specified ``from`` snapshot.
2288+
this for method chaining
23102289
"""
23112290
return self.update(from_snapshot_id_exclusive=from_snapshot_id_exclusive)
23122291

23132292
def to_snapshot_inclusive(self: IAS, to_snapshot_id_inclusive: int | None) -> IAS:
2314-
"""Instruct this scan to look for changes up to a particular snapshot (inclusive).
2293+
"""Instructs this scan to look for changes up to a particular snapshot (inclusive).
23152294
23162295
Args:
2317-
to_snapshot_id_inclusive: the end snapshot ID (inclusive).
2296+
to_snapshot_id_inclusive: the end snapshot ID (inclusive)
23182297
23192298
Returns:
2320-
A copy of this scan with the specified ``to`` snapshot.
2299+
this for method chaining
23212300
"""
23222301
return self.update(to_snapshot_id_inclusive=to_snapshot_id_inclusive)
23232302

@@ -2328,11 +2307,7 @@ def projection(self) -> Schema:
23282307
return current_schema.select(*self.selected_fields, case_sensitive=self.case_sensitive)
23292308

23302309
def plan_files(self) -> Iterable[FileScanTask]:
2331-
"""Plan the relevant files added between the specified snapshots.
2332-
2333-
Returns:
2334-
An iterable of FileScanTasks for data files added by append snapshots in the configured range.
2335-
"""
2310+
"""Plans the relevant files added between the specified snapshots."""
23362311
from_snapshot_id, to_snapshot_id = self._validate_and_resolve_snapshots()
23372312

23382313
append_snapshots = [
@@ -2425,7 +2400,7 @@ def _validate_and_resolve_snapshots(self) -> tuple[int, int]:
24252400

24262401
# The start snapshot is exclusive, so it does not need to be present in the table metadata
24272402
# (it may have been expired). It is valid as long as it is the parent of some ancestor of
2428-
# the end snapshot. This matches the behavior of `BaseIncrementalScan` in iceberg-java.
2403+
# the end snapshot.
24292404
if not is_parent_ancestor_of(to_snapshot_id, self.from_snapshot_id_exclusive, self.table_metadata):
24302405
raise ValueError(
24312406
f"Starting snapshot (exclusive) {self.from_snapshot_id_exclusive} is not a parent "

pyiceberg/table/snapshots.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -457,12 +457,7 @@ def ancestors_between_ids(
457457

458458

459459
def is_parent_ancestor_of(snapshot_id: int, ancestor_parent_snapshot_id: int, table_metadata: TableMetadata) -> bool:
460-
"""Return whether any ancestor of ``snapshot_id`` has ``ancestor_parent_snapshot_id`` as its parent.
461-
462-
This is used to validate an exclusive "from" snapshot for an incremental scan: the start snapshot
463-
is exclusive, so it only needs to be the parent of some ancestor of the end snapshot, not an ancestor
464-
itself. This admits start snapshots that have since been expired from the table metadata.
465-
"""
460+
"""Return whether any ancestor of ``snapshot_id`` has ``ancestor_parent_snapshot_id`` as its parent."""
466461
snapshot = table_metadata.snapshot_by_id(snapshot_id)
467462
if snapshot is None:
468463
return False

tests/table/test_init.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ def test_incremental_append_scan_invalid_to_snapshot(table_v2: Table) -> None:
395395
(42, 3055729675574597004, "Starting snapshot .exclusive. 42 is not a parent ancestor"),
396396
# 'from' and 'to' reversed (newer used as exclusive start).
397397
(3055729675574597004, 3051729675574597004, "is not a parent ancestor of end snapshot"),
398-
# Equal from/to: a snapshot is never its own parent ancestor (matches iceberg-java).
398+
# Equal from/to: a snapshot is never its own parent ancestor.
399399
(3055729675574597004, 3055729675574597004, "is not a parent ancestor of end snapshot"),
400400
],
401401
)

0 commit comments

Comments
 (0)