Skip to content

[python] Improve catalog API type annotations - #10025

Merged
JingsongLi merged 2 commits into
apache:masterfrom
wangzhigang1999:fix/python-catalog-type-hints
Sep 21, 2026
Merged

JingsongLi merged 2 commits into
apache:masterfrom
wangzhigang1999:fix/python-catalog-type-hints

Conversation

@wangzhigang1999

Copy link
Copy Markdown
Contributor

Purpose

Fixes #10024.

Callers lose static type information at Catalog.get_table(): Pyright reports Unknown for the returned table and subsequent read-builder calls because Catalog does not import the declared return type.

Add missing type imports and parameter/return annotations to Catalog, and align the filesystem, JDBC and REST implementations. Also align the optional database-property type through RESTApi.create_database() and CreateDatabaseRequest.

With these declarations, callers can resolve Table, read/write builders, snapshots, partitions and tags. The patch covers six files and preserves the existing operation bodies apart from moving imports to module scope.

Tests

  • 77 existing tests passed locally, covering catalog operations, tags, system-table dispatch, builders, overwrite commits and REST API parameter validation.
  • Verified the calling code with Pyright and six definition lookups with Jedi. Table and builder types resolve after the change.
  • Imported 12 related modules in separate Python processes and resolved all Catalog method annotations with typing.get_type_hints().
  • Verified unchanged database-request JSON for None, empty and populated properties. Flake8 and git diff --check passed.

@wangzhigang1999
wangzhigang1999 marked this pull request as ready for review September 20, 2026 15:26
Copilot AI lite review requested due to automatic review settings September 20, 2026 15:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The reviewed annotations are consistent with existing implementations and preserve runtime behavior.

Review effort: Lite
Findings: None

What changed in this PR

Improves static type information across the Python catalog API and implementations.

Changes:

  • Adds concrete catalog, table, snapshot, partition, tag, and authentication annotations.
  • Aligns filesystem, JDBC, and REST catalog signatures.
  • Makes REST database properties consistently optional.
File Description
catalog/​catalog.py Defines typed catalog API contracts.
catalog/​filesystem_catalog.py Aligns filesystem catalog annotations.
catalog/​jdbc_catalog.py Aligns JDBC catalog annotations.
catalog/​rest/​rest_catalog.py Adds REST catalog annotations and imports.
api/​rest_api.py Accepts optional database properties.
api/​api_request.py Types database request options as optional.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@JingsongLi JingsongLi 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.

Requirement fit: SUPPORTED (triage: GO)\nImplementation: CLEAN\n\nThe static contract has end-to-end developer value: Catalog.get_table and the downstream table, builder, snapshot, partition, and tag APIs now remain typed across the supported catalog implementations. I checked the moved runtime imports for cycles and the optional database-property serialization path; I found no actionable P0/P1/P2 issue in the current diff.

@JingsongLi
JingsongLi merged commit 8c607a5 into apache:master Sep 21, 2026
12 of 14 checks passed
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.

[Feature] Improve PyPaimon catalog API type annotations

3 participants