Enable doc tests in local and CI testing#1409
Conversation
…pised pre-commit didn't flag for anyone else
There was a problem hiding this comment.
Pull request overview
This PR enables doctests in the DataFusion Python library to ensure docstring examples are validated both locally and in CI. It is Part 1 of a larger effort (#1400) to improve developer and agent usability through better documentation and tested examples.
Changes:
- Enables
--doctest-modulesin pytest configuration (pyproject.toml) and setstestpathsto scope collection topython/testsandpython/datafusion - Updates the GitHub Actions
test.ymlto fix a broken cache key reference and removes the explicit.path from the pytest command (deferring totestpaths) - Fixes two existing docstring examples in
dataframe.py(into_viewandfill_null) that were pseudo-code or would fail as doctests
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pyproject.toml |
Enables --doctest-modules, adds doctest_optionflags, and sets testpaths to scope doctest collection |
.github/workflows/test.yml |
Fixes broken cargo cache key (steps.rust-toolchain.outputs.cachekey → matrix.toolchain) and removes explicit . path from pytest command |
python/datafusion/dataframe.py |
Fixes into_view and fill_null docstring examples to be valid, self-contained, and executable doctests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [tool.pytest.ini_options] | ||
| asyncio_mode = "auto" | ||
| asyncio_default_fixture_loop_scope = "function" | ||
| addopts = "--doctest-modules" |
There was a problem hiding this comment.
| >>> df = df.fill_null(0) # Fill all nulls with 0 where possible | ||
| >>> # Fill nulls in specific string columns | ||
| >>> df = df.fill_null("missing", subset=["name", "category"]) | ||
| >>> from datafusion import SessionContext, col |
There was a problem hiding this comment.
Thanks @ntjohnson1
Did you verify from the CI run logs that these tests are now run? I took a brief look and couldn't find anything that was new https://github.com/apache/datafusion-python/actions/runs/22687531822/job/65776433980?pr=1409
🤔
There was a problem hiding this comment.
The doctests run as a part of pytest so if you click on Run Tests you can see the new doctest entries.
There aren't that many examples in the code at the moment.
If you scroll all the way to the bottom you can now see source code showing up in addition to test code
python/datafusion/dataframe.py::datafusion.dataframe.DataFrame.fill_null PASSED [ 99%]
python/datafusion/dataframe.py::datafusion.dataframe.DataFrame.into_view PASSED [ 99%]
python/datafusion/dataframe_formatter.py::datafusion.dataframe_formatter.configure_formatter PASSED [ 99%]
python/datafusion/dataframe_formatter.py::datafusion.dataframe_formatter.get_formatter PASSED [ 99%]
python/datafusion/dataframe_formatter.py::datafusion.dataframe_formatter.reset_formatter PASSED [ 99%]
python/datafusion/dataframe_formatter.py::datafusion.dataframe_formatter.set_formatter PASSED [100%]There was a problem hiding this comment.
Nice -- I checked https://github.com/apache/datafusion-python/actions/runs/22676065624/job/65741468321 (a recent run from main) and indeed those tests are not there
python/tests/test_view.py::test_register_filtered_dataframe PASSED [ 99%]
python/tests/test_wrapper_coverage.py::test_datafusion_missing_exports PASSED [100%]
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Makes sense to me -- thanks @ntjohnson1
@timsaucer any concern with merging this one?
|
I am going to say let's get this party started! |
Which issue does this PR close?
Rationale for this change
This enables doctests so if more examples are added to doc strings they can be validated locally and in CI.
What changes are included in this PR?
Are there any user-facing changes?
No