Skip to content

fix: make csv-read-options.py example self-contained (#1728) - #1729

Open
dedsec-terminal wants to merge 4 commits into
apache:mainfrom
dedsec-terminal:fix-csv-read-options-example
Open

fix: make csv-read-options.py example self-contained (#1728)#1729
dedsec-terminal wants to merge 4 commits into
apache:mainfrom
dedsec-terminal:fix-csv-read-options-example

Conversation

@dedsec-terminal

Copy link
Copy Markdown

Which issue does this PR close?

Closes #1728.

Rationale for this change

examples/csv-read-options.py failed when executed because it attempted to read data.csv and data.csv.gz which do not exist in the repository.

What changes are included in this PR?

  • Made examples/csv-read-options.py self-contained by generating small temporary CSV and gzipped CSV files in a tempfile.TemporaryDirectory().
  • Added .show() calls to display example DataFrame outputs.

Are there any user-facing changes?

No breaking changes; fixes example execution so it is runnable out of the box.

Copilot AI lite review requested due to automatic review settings September 11, 2026 14:35

Copilot AI 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.

🟡 Changes recommended

The delimiter mismatch affects example correctness, and Example 3 does not execute.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes examples/csv-read-options.py by making it runnable with generated temporary CSV and gzip inputs.

Changes:

  • Generates temporary CSV fixtures.
  • Updates reads and registrations to use them.
  • Adds output display calls.
File summaries
File Summary
examples/csv-read-options.py Self-contained CSV options demonstrations. Moderate finding (1 vote): delimiter does not match the generated fixture. Nit (3 votes): Example 3 needs a terminal call such as df.show().
Review details

Suppressed comments (1)

examples/csv-read-options.py:64

  • The generated fixture is comma-separated, but this example configures | as the delimiter. It therefore reads each row as one field instead of demonstrating a valid custom-delimiter CSV; use a pipe-delimited fixture for this example or change the delimiter to match the fixture.
        .with_delimiter("|")
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread examples/csv-read-options.py Outdated
Signed-off-by: dedsec-terminal <dedsec.terminal@gmail.com>
Signed-off-by: dedsec-terminal <dedsec.terminal@gmail.com>
@dedsec-terminal

Copy link
Copy Markdown
Author

Follow-up: I executed the example end to end and found one more runtime edge case: the literal null was inferred as an integer and rejected during parsing. Commit 9e001287 now uses an empty CSV field for the sample null value. All six examples complete successfully with DataFusion 54.0.0, and Ruff 0.15.1 lint/format checks pass. Ready for review.

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