Skip to content

fix(python): handle missing manifest partition summaries - #2886

Open
mattfaltyn wants to merge 2 commits into
apache:mainfrom
mattfaltyn:agent/fix-python-manifest-partitions
Open

fix(python): handle missing manifest partition summaries#2886
mattfaltyn wants to merge 2 commits into
apache:mainfrom
mattfaltyn:agent/fix-python-manifest-partitions

Conversation

@mattfaltyn

@mattfaltyn mattfaltyn commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

What changes are included in this PR?

The Python ManifestFile.partitions getter now returns None when the optional core partition summaries are absent. Previously, the getter unconditionally unwrapped the optional value and triggered a Rust/PyO3 panic for valid manifest entries with partitions: None.

The optional return type preserves the distinction between absent summaries and a present empty list, avoids cloning the entire summaries vector before converting its entries, and a focused regression test covers the absent-summary case.

Are these changes tested?

Yes. The regression test constructs a valid ManifestFile with partitions: None and verifies that the getter returns None.

  • cargo test --locked -p pyiceberg_core_rust test_manifest_partitions_without_summaries -- --nocapture
  • cargo fmt --all -- --check
  • cargo clippy --locked -p pyiceberg_core_rust --all-targets --all-features -- -D warnings

@mattfaltyn
mattfaltyn marked this pull request as ready for review July 24, 2026 07:02
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

@github-actions github-actions Bot added the stale label Aug 24, 2026
Comment thread bindings/python/src/manifest.rs

@dannycjones dannycjones left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I think we should consider changing the return type here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python ManifestFile.partitions panics when partition summaries are absent

2 participants