feat: run examples/*.py in CI and fix the examples that could not run - #1732
feat: run examples/*.py in CI and fix the examples that could not run#1732Developer1010x wants to merge 1 commit into
Conversation
No workflow executed any top-level `examples/*.py`, so two problems had gone unnoticed: `csv-read-options.py` raised on its first read because the `data.csv` and `data.csv.gz` it reads are not in the repository, and nine scripts printed nothing at all, ending in a bare `assert`. - `csv-read-options.py` now writes its own CSV, pipe-separated CSV and gzipped CSV into a temporary directory, and shows the result of each example. - `export.py`, `import.py`, `python-udaf.py`, `python-udf.py`, `query-pyarrow-data.py`, `sql-to-pandas.py`, `sql-using-python-udaf.py`, `sql-using-python-udf.py` and `substrait.py` print their results, keeping the existing asserts. - `substrait.py` resolves its submodule data relative to `__file__` instead of the working directory. - A `Run Python examples` step runs the examples from the repository root after the TPC-H data is generated, skipping the five that need a manual download, a Ray cluster, or AWS credentials. - `examples/README.md` documents the per-example prerequisites and drops the links to `sql-on-polars.py`, `sql-on-pandas.py` and `sql-on-cudf.py`, which are not in the repository. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Filed the |
|
We have 3 PRs all addressing the same issue and all 3 appear to be just AI generated results.
I am tempted to just close all 3. If any of the 3 of you can suggest why one should be taken over another, I'd be interested to hear your thoughts. These mostly appear to be a single output of dumping the issue into an agent. |
|
Fair question, and I would rather answer it straight than dance around it: this was written with an AI agent. The commit carries a The distinction I would offer is not "mine is not AI" but that it was not one pass over the issue text. Against #1736, which is the closest of the three:
One point where #1736 is better than mine: it gates on On "a single output of dumping the issue into an agent": the thing that argues against that, for whatever it is worth, is what running the examples turned up. Closing all three is a perfectly reasonable call and I will not take it badly. If it is more useful, I am glad to cut this down to just the CI job, with their gate, so there is a smaller change for someone to review properly. |
|
The response above also appears generated. Closing in accordance with our parent project's AI usage guidance. https://datafusion.apache.org/contributor-guide/index.html#ai-assisted-contributions |
Which issue does this PR close?
Closes #1728.
Rationale for this change
Nothing in CI ran the top-level
examples/*.py, so nothing noticed when theystopped working. Two consequences were already in the tree, and both are the
first thing a new user copies from.
What changes are included in this PR?
examples/csv-read-options.pyno longer crashes. It readdata.csvanddata.csv.gz, neither of which is in the repository, so it raised on the firstread. It now writes what it needs into a temporary directory: a comma-separated
CSV, a pipe-separated and single-quoted one so the builder-pattern example
actually exercises the delimiter and quote it sets, and a gzipped one with a
comment line so the advanced-options example exercises
with_commentandwith_file_compression_type. Every example now callsdf.show(), so the scriptdemonstrates the options instead of only naming them.
Nine scripts printed nothing.
export.py,import.py,python-udaf.py,python-udf.py,query-pyarrow-data.py,sql-to-pandas.py,sql-using-python-udaf.py,sql-using-python-udf.pyandsubstrait.pyendedin a bare
assert. They now print their results; the asserts are unchanged.substrait.pyonly worked from the repository root. It read./testing/data/csv/aggregate_test_100.csv, so it failed from anywhere else. Itnow resolves that path relative to
__file__.The CI job. A
Run Python examplesstep, gated toabi3like the FFI andTPC-H steps, running from the repository root against the wheel built earlier in
the job. Placement and content are both load-bearing:
python-udf-comparisons.pyreadsexamples/tpch/data/lineitem.parquet. Theissue did not list this one; it shows up as soon as you actually run them.
pandasandpolars, which are neither runtime nor devdependencies but are what
import.pyandexport.pydemonstrate convertingto and from.
git submodule update --initalready done by the test stepfor
substrait.py.sql-parquet.py,dataframe-parquet.pyandsql-to-pandas.pyneed the NYC taxi Parquet filethe README says to download by hand;
ray_pickle_expr.pyneeds a Ray cluster;sql-parquet-s3.pyneeds network access and AWS credentials.every failure at once, so one broken script does not hide the next.
examples/README.mdgains a short "Running the examples" section with atable of the per-example prerequisites, and drops the links to
sql-on-polars.py,sql-on-pandas.pyandsql-on-cudf.py, which are not inthe repository.
Verification
All 14 non-skipped examples were run from the repository root against a
datafusion54.0.0 wheel, with the TPC-H data generated and thetestingsubmodule initialised. Every one exits 0 and prints output.
ruff check,ruff format --checkandcodespellpass, andshellcheckis clean on the newstep's script.
One thing found on the way, not fixed here
CsvReadOptions.with_null_regexappears to have no effect at read time. Withdatafusion54.0.0:N/Acomes back as the literal string rather than NULL, and if the same valuesits in a column inferred as
Int64the read fails outright withParser error: Error while parsing value 'N/A' as type 'Int64'. The option isplumbed through
crates/core/src/options.rsinto DataFusion'sCsvReadOptions,so this looks like it is below this crate. To keep this PR runnable and honest,
the advanced-options example keeps
with_null_regexset and puts theN/Ain astring column. Happy to open a separate issue for it.
Are there any user-facing changes?
No API changes. The examples print their results now, and
examples/csv-read-options.pyno longer requires data files that were nevershipped.