-
Notifications
You must be signed in to change notification settings - Fork 147
Add doctests to functions to help humans and agents #1400
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
d30797e
308c36a
cfa4ec9
17b009c
683158a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,6 +70,9 @@ features = ["substrait"] | |
| [tool.pytest.ini_options] | ||
| asyncio_mode = "auto" | ||
| asyncio_default_fixture_loop_scope = "function" | ||
| addopts = "--doctest-modules" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this here? (sorry I don't know python enough)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By default the examples in the doc strings aren't executed. There are a few different ways to turn that functionality on and this seemed the least intrusive. I broke down my commits somewhat (besides the giant here are loads of examples). So the first commit allows testing of the examples in the doc strings. Then I ran pytest (which now automatically runs the tests in the doc strings) and fixed the few cases we already had. |
||
| doctest_optionflags = ["NORMALIZE_WHITESPACE", "ELLIPSIS"] | ||
| testpaths = ["python/tests", "python/datafusion"] | ||
|
|
||
| # Enable docstring linting using the google style guide | ||
| [tool.ruff.lint] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems unrelated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICT this was already broken but my change here invalidated the cache and exercised the fact that this line is no longer valid. This was required to make CI, happy.