Skip to content

docs: document that Expr output method kwargs are backend-specific - #12085

Open
fbossiere wants to merge 1 commit into
ibis-project:mainfrom
fbossiere:docs/issue-11769-to-csv-backend-kwargs
Open

docs: document that Expr output method kwargs are backend-specific#12085
fbossiere wants to merge 1 commit into
ibis-project:mainfrom
fbossiere:docs/issue-11769-to-csv-backend-kwargs

Conversation

@fbossiere

Copy link
Copy Markdown

Description of changes

Table.to_csv documents its **kwargs as "Additional keyword arguments passed to pyarrow.csv.CSVWriter", but the expression-level method just dispatches to the backend:

self._find_backend(use_default=True).to_csv(self, path, params=params, **kwargs)

Only the default BaseBackend implementation goes through PyArrow. BackendDuckDB.to_csv overrides it and documents its own kwargs as "DuckDB CSV writer arguments" — and DuckDB is the default backend, so the documented PyArrow contract is wrong on the most common path.

The same shape applies to the sibling output methods, so this PR fixes the whole class rather than just the reported method:

Expr method overridden by
to_csv DuckDB
to_parquet DuckDB, PySpark
to_parquet_dir PySpark
to_delta PySpark

For each of the four, **kwargs is now described as "Additional, backend-specific keyword arguments." — the wording to_json already uses, and which #11769 asks for. The PyArrow / deltalake references are not dropped: they move into a Notes section as the default implementation's behaviour, with a pointer to the backend's own method. This follows the precedent of #8623 (to_delta kwargs docs).

to_json and to_xlsx already described their kwargs correctly and are left alone.

Testing

Documentation-only change — no executable behaviour is modified, so there is no regression test to add. Verified:

  • ruff check --force-exclude ibis/expr/types/core.py → passes
  • ruff format --force-exclude --check ibis/expr/types/core.py → already formatted
  • codespell ibis/expr/types/core.py → clean
  • docstrings re-parsed via ast.get_docstring to confirm the numpydoc sections are well-formed; Notes is placed before Examples, matching to_xlsx in the same file
  • the one new URL (https://delta-io.github.io/delta-rs/api/delta_writer/) returns HTTP 200, for the lychee link check

Deliberately left out

  • No change to the backend implementations or to any signature.
  • No change to to_json / to_xlsx, whose wording is already correct.
  • The per-backend writer options are not enumerated in the base docstrings; they belong with each backend's own method and would drift.

Issues closed

The expression-level to_csv/to_parquet/to_parquet_dir/to_delta methods
dispatch to the backend, so their **kwargs are whatever that backend's
writer accepts -- not unconditionally the PyArrow writer's arguments.
DuckDB's to_csv takes DuckDB CSV writer options and DuckDB/PySpark
override to_parquet, to_parquet_dir and to_delta as well, so the old
wording was wrong on the default backend.

Describe kwargs as backend-specific (matching to_json) and keep the
PyArrow/deltalake references in a Notes section as the default
implementation's behaviour.

Resolves ibis-project#11769
@deepyaman deepyaman added the docs-preview Add this label to trigger a docs preview label Aug 25, 2026
@deepyaman

Copy link
Copy Markdown
Collaborator

@fbossiere Thanks for the interest in contributing! Looks reasonable at a glance, but can you first look at the docs preview (I've run it on https://github.com/ibis-project/ibis/actions/runs/32811654581/job/97691967925?pr=12085) and ensure it renders as expected on the base classes as well as the backends? Some screenshots from the docs build wouldn't hurt.

I'll take a proper look after that sanity check!

@deepyaman
deepyaman self-requested a review August 25, 2026 05:11
@ibis-docs-bot ibis-docs-bot Bot removed the docs-preview Add this label to trigger a docs preview label Aug 25, 2026
@fbossiere

Copy link
Copy Markdown
Author

Thanks for the quick look, @deepyaman! Two things: the rendering check you asked for, and a heads-up about the preview link.

⚠️ The docs preview never actually built

The run you linked failed before it could render anything, and it's not related to this PR:

##[error]Refusing to check out fork pull request code from a
'pull_request_target' workflow. This workflow runs with the base
repository's GITHUB_TOKEN, secrets, default-branch cache scope, and
runner access. […] To opt in, review the risks at
https://gh.io/securely-using-pull_request_target and set
'allow-unsafe-pr-checkout: true' on the actions/checkout step.

docs-preview.yml triggers on pull_request_target, and actions/checkout@v6
now refuses by default to check out fork code in that trusted context
("pwn request" hardening).
Every other recent run of that workflow is skipped, so mine looks like the
first one to reach the checkout step and hit the new guard — this probably
affects all external contributor PRs, not just this one. Might be worth a
separate issue.

So I built the docs locally instead (quartodoc build + quarto render,
clean exit) and screenshotted the result.

Base class — Expr methods on the Table reference page

All four changed methods render as intended: the **kwargs row reads
"Additional, backend-specific keyword arguments", and the PyArrow reference
moved into a Notes section that keeps the original link. to_xlsx is
included as an untouched control — same wording style, same section order.

image

Backends — unchanged, as expected

This is the part worth being precise about, since it's what the issue is really
about. Backends that provide their own writer override these methods, so
their own docstrings still win and this PR doesn't touch them. Rendered
docs/backends/duckdb.qmd locally:

image2

duckdb.Backend.to_csv still documents kwargs as "DuckDB CSV writer
arguments" with the DuckDB link, and to_parquet likewise — the generic
"backend-specific" wording appears zero times on that page. pyspark.Backend
overrides to_csv, to_parquet, to_parquet_dir and to_delta the same way.

One thing I deliberately did NOT change

_FileIOHandler in ibis/backends/__init__.py also documents these methods and
names PyArrow explicitly. That wording is correct there and I left it alone:
that class is the default PyArrow implementation
(to_csvpyarrow.csv.CSVWriter, to_parquetParquetWriter,
to_parquet_dirwrite_dataset, to_deltawrite_deltalake). The problem
was only on Expr, which merely delegates to whichever backend is active — so
promising PyArrow there is wrong for DuckDB, the default backend, which is
exactly what #11769 reports.

Two small pre-existing nits I noticed but left out to keep the diff scoped —
say the word and I'll fold either in (or file them separately):

  • _FileIOHandler.to_json has a typo: backend-specifcbackend-specific
  • these four methods document the parameter as **kwargs while most of the
    file uses kwargs (10 vs 4 occurrences); the **kwargs spelling predates
    this PR

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.

docs: Table.to_csv documentation not updated

2 participants