Skip to content

feat: run examples/*.py in CI and fix the examples that could not run - #1732

Closed
Developer1010x wants to merge 1 commit into
apache:mainfrom
Developer1010x:run-examples-in-ci
Closed

Developer1010x wants to merge 1 commit into
apache:mainfrom
Developer1010x:run-examples-in-ci

Conversation

@Developer1010x

Copy link
Copy Markdown

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 they
stopped 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.py no longer crashes. It read data.csv and
data.csv.gz, neither of which is in the repository, so it raised on the first
read. 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_comment and
with_file_compression_type. Every example now calls df.show(), so the script
demonstrates 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.py and substrait.py ended
in a bare assert. They now print their results; the asserts are unchanged.

substrait.py only worked from the repository root. It read
./testing/data/csv/aggregate_test_100.csv, so it failed from anywhere else. It
now resolves that path relative to __file__.

The CI job. A Run Python examples step, gated to abi3 like the FFI and
TPC-H steps, running from the repository root against the wheel built earlier in
the job. Placement and content are both load-bearing:

  • It runs after the TPC-H generation step, because
    python-udf-comparisons.py reads examples/tpch/data/lineitem.parquet. The
    issue did not list this one; it shows up as soon as you actually run them.
  • It installs pandas and polars, which are neither runtime nor dev
    dependencies but are what import.py and export.py demonstrate converting
    to and from.
  • It relies on the git submodule update --init already done by the test step
    for substrait.py.
  • It skips five, each for a stated reason in the workflow: sql-parquet.py,
    dataframe-parquet.py and sql-to-pandas.py need the NYC taxi Parquet file
    the README says to download by hand; ray_pickle_expr.py needs a Ray cluster;
    sql-parquet-s3.py needs network access and AWS credentials.
  • A failing example does not stop the loop. All of them run and the step reports
    every failure at once, so one broken script does not hide the next.

examples/README.md gains a short "Running the examples" section with a
table of 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.

Verification

All 14 non-skipped examples were run from the repository root against a
datafusion 54.0.0 wheel, with the TPC-H data generated and the testing
submodule initialised. Every one exits 0 and prints output. ruff check,
ruff format --check and codespell pass, and shellcheck is clean on the new
step's script.

One thing found on the way, not fixed here

CsvReadOptions.with_null_regex appears to have no effect at read time. With
datafusion 54.0.0:

from pathlib import Path
from datafusion import CsvReadOptions, SessionContext

p = Path("probe.csv")
p.write_text("id,name,value\n1,alice,10\n2,N/A,20\n3,carol,30\n")
ctx = SessionContext()
options = CsvReadOptions().with_has_header(True).with_null_regex(r"^(null|NULL|N/A)$")
ctx.read_csv(p, options=options).show()

N/A comes back as the literal string rather than NULL, and if the same value
sits in a column inferred as Int64 the read fails outright with
Parser error: Error while parsing value 'N/A' as type 'Int64'. The option is
plumbed through crates/core/src/options.rs into DataFusion's CsvReadOptions,
so this looks like it is below this crate. To keep this PR runnable and honest,
the advanced-options example keeps with_null_regex set and puts the N/A in a
string 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.py no longer requires data files that were never
shipped.

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>
@Developer1010x

Copy link
Copy Markdown
Author

Filed the null_regex finding mentioned above as #1735 — it turned out to be upstream in datafusion-datasource-csv, where the reader's ReaderBuilder never gets with_null_regex. Nothing in this PR depends on it.

@timsaucer

Copy link
Copy Markdown
Member

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.

@Developer1010x

Copy link
Copy Markdown
Author

Fair question, and I would rather answer it straight than dance around it: this was written with an AI agent. The commit carries a Co-Authored-By: Claude trailer for that reason, so you are not wrong about any of the three.

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:

  • Coverage. My step runs after the tpchgen-cli step, so python-udf-comparisons.py is exercised rather than skipped. It reads examples/tpch/data/lineitem.parquet, which that step creates. Run examples/*.py in CI, fix the broken example, and give silent ones output #1736 places the job before it and skips the example for that reason. 14 examples run instead of 13. The issue does not mention this dependency, it only shows up once you actually run them.
  • substrait.py reads ./testing/data/csv/aggregate_test_100.csv, so it only works when the working directory is the repository root. I resolve it relative to __file__. Both approaches pass in CI; only one works when a user runs the file directly.
  • Example 3 of csv-read-options.py configures delimiter="|" and quote="'". fix: show results in the nine silent top-level examples #1730 and Run examples/*.py in CI, fix the broken example, and give silent ones output #1736 point it at the comma-separated fixture, so it runs green and prints a single-column table, demonstrating the opposite of what it sets. I write a pipe-delimited file for that example and a gzipped one with a comment line for the advanced-options example.
  • examples/README.md links sql-on-polars.py, sql-on-pandas.py and sql-on-cudf.py. None of the three are in the repository. Removed, and replaced with a table of what each example needs.

One point where #1736 is better than mine: it gates on matrix.python-version == '3.12', which runs the job once. I followed the neighbouring matrix.wheel-tag == 'abi3' convention, and that matches five matrix rows, so my job runs five times where once is enough. Their gate is the correct one and I am happy to switch to it.

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. CsvReadOptions.with_null_regex is accepted and silently ignored (#1735). That traced to datafusion-datasource-csv, where CsvSource::builder never passes the regex to the ReaderBuilder that parses the rows, so it applies during schema inference only: apache/datafusion#25213, fix in apache/datafusion#25254. Chasing that turned up a second, unrelated defect on the same path, where an invalid pattern panics the query task instead of returning an error: apache/datafusion#25260, fix in apache/datafusion#25261. None of that is in issue #1728.

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.

@timsaucer

Copy link
Copy Markdown
Member

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

@timsaucer timsaucer closed this Sep 14, 2026
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.

Nothing in CI runs examples/*.py, and one of them is broken

2 participants