Make scan_plan_helper internal (#3541) by tanmayrauth · Pull Request #3553 · apache/iceberg-python · GitHub
Skip to content

Make scan_plan_helper internal (#3541)#3553

Merged
Fokko merged 1 commit into
apache:mainfrom
tanmayrauth:make-scan-plan-helper-internal-3541
Jun 28, 2026
Merged

Make scan_plan_helper internal (#3541)#3553
Fokko merged 1 commit into
apache:mainfrom
tanmayrauth:make-scan-plan-helper-internal-3541

Conversation

@tanmayrauth

@tanmayrauth tanmayrauth commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

scan_plan_helper is an internal helper for local scan planning, not a public extension point. Rename it to _scan_plan_helper to reflect its intended visibility and update its only caller in inspect.py.

The method was introduced recently in d99936a and has not been part of any released public API, so no deprecation shim is required.
Fixes: #3541

Rationale for this change

Are these changes tested?

Are there any user-facing changes?

scan_plan_helper is an internal helper for local scan planning, not a
public extension point. Rename it to _scan_plan_helper to reflect its
intended visibility and update its only caller in inspect.py.

The method was introduced recently in d99936a and has not been part of
any released public API, so no deprecation shim is required.
@ebyhr

ebyhr commented Jun 23, 2026

Copy link
Copy Markdown
Member

@rambleraptor rambleraptor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Easy peasy. Thanks for the PR!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: I feel _helper suffix is weird. The name should describe what it does, not that it's a helper. We could rename it to _plan_manifest_entries or something.

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.

Let's make it internal first, we can always change the name then :)

@Fokko Fokko merged commit 6c3c2f3 into apache:main Jun 28, 2026
17 checks passed
kevinjqliu pushed a commit that referenced this pull request Jun 29, 2026
# Rationale for this change

The method name should describe what it does. 

- Follow-up of #3553

## Are these changes tested?

Yes

## Are there any user-facing changes?

No, this is an internal method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make scan_plan_helper internal

5 participants