8000 Update annotation with V3 by Fokko · Pull Request #1587 · apache/iceberg-python · GitHub
[go: up one dir, main page]
Skip to content
  • Insights

    • Update annotation with V3 #1587

    New issue

    Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

    Sign up for GitHub

    By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

    Already on GitHub? Sign in to your account

    Jump to bottom
    Merged
    kevinjqliu merged 2 commits into from
    Jan 30, 2025
    Merged

    Update annotation with V3 #1587

    kevinjqliu merged 2 commits into from
    Jan 30, 2025

    Conversation

    @Fokko
    Copy link
    Contributor
    @Fokko Fokko commented Jan 28, 2025

    Missed this in #1554

    kevinjqliu
    kevinjqliu reviewed Jan 28, 2025
    View reviewed changes
    Copy link
    Contributor
    @kevinjqliu kevinjqliu left a comment

    Choose a reason for hiding this comment

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

    i swear i saw this was already done. turns our there are 2 dups!!

    iceberg-python/pyiceberg/table/metadata.py

    Line 568 in 1adbb87

    TableMetadata = Annotated[Union[TableMetadataV1, TableMetadataV2, TableMetadataV3], Field(discriminator="format_version")]

    @Fokko
    Copy link
    Contributor Author
    Fokko commented Jan 30, 2025

    @kevinjqliu Maybe you already saw this at the Deletion Vector PR: https://github.com/apache/iceberg-python/pull/1516/files#diff-a7fbd8c6b0564308ca872ba479f0e240ff2c893d15776a14e52b91b48e9ac470R693

    I noticed it over there that it was missing, but it isn't really part of the DV PR.

    @kevinjqliu
    Copy link
    Contributor
    kevinjqliu commented Jan 30, 2025 •
    edited
    Loading

    @Fokko I meant this change already exists on L568

    there are two

    TableMetadata = Annotated[Union[TableMetadataV1, TableMetadataV2, TableMetadataV3], Field(discriminator="format_version")]

    in pyiceberg/table/metadata.py

    was added here
    https://github.com/apache/iceberg-python/pull/1554/files#diff-a7fbd8c6b0564308ca872ba479f0e240ff2c893d15776a14e52b91b48e9ac470R576

    @Fokko
    Copy link
    Contributor Author
    Fokko commented Jan 30, 2025

    Ah good catch @kevinjqliu. Probably the type: ignore hid the error from the linter, thanks!

    kevinjqliu
    kevinjqliu approved these changes Jan 30, 2025
    View reviewed changes
    Copy link
    Contributor
    @kevinjqliu kevinjqliu left a comment

    Choose a reason for hiding this comment

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

    🪄

    @kevinjqliu kevinjqliu merged commit 37916c4 into main Jan 30, 2025
    7 checks passed
    @kevinjqliu kevinjqliu deleted the fd-follow-up-v3 branch January 30, 2025 18:38
    @kevinjqliu
    Copy link
    Contributor
    kevinjqliu commented Jan 30, 2025

    Already added in #1554

    @Fokko
    Copy link
    Contributor Author
    Fokko commented Jan 30, 2025

    Less is more, thanks @kevinjqliu 🙌

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

    Reviewers

    @kevinjqliu kevinjqliu kevinjqliu approved these changes

    Assignees

    No one assigned

    Labels

    None yet

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @Fokko @kevinjqliu
    0