Skip to content

[python] Split examples in Python and enforce executability in CI for python examples#610

Open
charlesdong1991 wants to merge 4 commits into
apache:mainfrom
charlesdong1991:python-example-ci
Open

[python] Split examples in Python and enforce executability in CI for python examples#610
charlesdong1991 wants to merge 4 commits into
apache:mainfrom
charlesdong1991:python-example-ci

Conversation

@charlesdong1991

Copy link
Copy Markdown
Contributor

Purpose

Code examples in the Python binding were never executed in CI, so they could silently rot and mislead users learning the system.
This splits the monolithic example into focused, standalone-runnable scripts and adds an automated executability check.

Linked issue: close #607

Tests

All tests passed locally

API and Format

Documentation

@charlesdong1991

Copy link
Copy Markdown
Contributor Author

@fresh-borzoni @leekeiabstraction let me know what you think 🙏

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

Thank you @charlesdong1991 for the PR, great idea.

QQ as I do not have access atm: with the proposed changes, what happens if i) method signatures doesn't match and II) assertion fails?

@fresh-borzoni fresh-borzoni left a comment

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.

@charlesdong1991 Ty for the PR, makes sense overall, let me merge test changes from python MAP/ROW PR first then

also shall we wire stubtest to test .pyi drift? Probably not in this PR, but since we are talking about drift in examples :)
WDYT?

@charlesdong1991

Copy link
Copy Markdown
Contributor Author

with the proposed changes, what happens if i) method signatures doesn't match and II) assertion fails?

assertion failures and method signatures mismatches will be caught at run time, to catch signature drift statically (including unexcercised ones), we can use .pyi drift testing... i also add assertions explicitly...

wdyt? @leekeiabstraction

also shall we wire stubtest to test .pyi drift?

yeah, we can do it, nice idea! there are 2 stub issues that are fixed in this PR, and wiring it to pass cleanly needs a bit more work as there are quite some diffs (a bit more than 100)... i can create a focused follow-up pr then if you are fine with that @fresh-borzoni

@fresh-borzoni

Copy link
Copy Markdown
Member

yeah, we can do it, nice idea! there are 2 stub issues that are fixed in this PR, and wiring it to pass cleanly needs a bit more work as there are quite some diffs (a bit more than 100)... i can create a focused follow-up pr then if you are fine with that @fresh-borzoni

@charlesdong1991 I'm fine with this 👍
Shall we add examples for additional complex types(map, rows) we merged in main? So that examples reflect the current capabilities?

@charlesdong1991

Copy link
Copy Markdown
Contributor Author

added! @fresh-borzoni thanks!

PTAL! 🙏 @leekeiabstraction @fresh-borzoni

@fresh-borzoni fresh-borzoni left a comment

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.

@charlesdong1991 Thank you, LGTM 👍

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.

[python] Update examples for python and enforce executable checks in CI for code examples

3 participants