diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..bb18259 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,9 @@ + diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..d85f8ec --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,53 @@ +# AGENTS.md + +Entry point for AI agents working in the `substrait-python` repository. Read the shared, +human-facing docs first, then keep the notes below in mind. + +## Start here + +- **[`README.md`](README.md)** — what the project is, the DataFrame API, and the lower-level + `substrait.proto` / `substrait.builders` layers. +- **[`CONTRIBUTING.md`](CONTRIBUTING.md)** — how the spec relates to this repo, the development + environment, the lint / format / test command mechanics, and the commit and PR conventions. + +This repo *implements* the Substrait spec; it does not define it. Read +[the specification is the source of truth](CONTRIBUTING.md#the-specification-is-the-source-of-truth) +before changing behavior. The failure mode to avoid is filling a gap in the spec with something +plausible and then describing it as spec-defined. When you cannot find the spec's answer, say so +explicitly instead of picking one silently: check the sibling bindings listed at +[Active Libraries](https://substrait.io/community/active_libraries/) for an existing consensus, and +surface what is still unresolved in the PR. + +The proto bindings, the standard extension YAMLs, and the ANTLR grammar are **not** vendored here — +they come from the `substrait-protobuf`, `substrait-extensions`, and `substrait-antlr` +distributions pinned in [`pyproject.toml`](pyproject.toml), which is what ties this tree to a spec +version (reported at runtime by `substrait.version.substrait_version`). Changing one of those +inputs means changing the spec, not this repo. + +## Conventions & workflow + +- **Keep PR descriptions high-signal.** The PR title and body together become the squash-merge + commit message that `semantic-release` uses to build [`CHANGELOG.md`](CHANGELOG.md) — the body is + changelog input, not a review scratchpad. Follow + [`CONTRIBUTING.md`](CONTRIBUTING.md#pull-requests) rather than + [`.github/pull_request_template.md`](.github/pull_request_template.md), which a PR opened with an + explicitly supplied body never shows you. Beyond forming a valid conventional commit, leave out + the noise agents tend to add: + - **Lists of files touched** — they're in the diff. + - **Claims that CI-verified things pass** — e.g. "tests pass", "ruff clean". If they didn't, the + checks would be red. + - **Process notes that are already implicit** — e.g. "opened as draft pending review". + + Do include the rationale, and for spec-tracking changes the spec version (e.g. `spec v0.99.0`). + Keep commit bodies free of git trailers (`Signed-off-by`, `Co-authored-by`, tool-attribution + lines) — `semantic-release` builds the changelog from the commit message, and history here does + not carry them. +- **A `BREAKING CHANGE:` footer goes last, with nothing after it**, and the title gets a `!` + (`feat!: …`). The footer text *is* the published ⚠ BREAKING CHANGES note, and the + conventional-commits parser ends that note only at another footer keyword or an issue reference, + so any other trailing line is absorbed into it and published verbatim. Prose under a + `## Breaking change` heading is not a footer and never reaches the release notes at all; see + [`CONTRIBUTING.md`](CONTRIBUTING.md#breaking-changes). +- **Run `uv run pytest`, `pixi run lint`, and `pixi run format --check` before pushing.** All three + run in CI, and the test job runs the matrix with `--frozen`, so a dependency change has to land + with its lock files regenerated. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..43c994c --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9ac1315..5847286 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,18 +1,51 @@ -# Getting Started -## Get the repo +# Contributing to Substrait Python + +This page provides some orientation and recommendations on how to get the best results when engaging with the community. + +1. [The specification is the source of truth](#the-specification-is-the-source-of-truth) +2. [Getting started](#getting-started) +3. [Lint & format](#lint--format) +4. [Testing](#testing) +5. [Commit conventions](#commit-conventions) +6. [Pull requests](#pull-requests) + +## The specification is the source of truth + +Substrait Python is an implementation of the [Substrait specification](https://substrait.io/); it does not define Substrait semantics. Review behavioral changes against the spec — the spec text and the `.proto` comments in [`substrait-io/substrait`](https://github.com/substrait-io/substrait) for the version this tree targets. + +That version is pinned by the `substrait-protobuf`, `substrait-extensions` and `substrait-antlr` requirements in [`pyproject.toml`](pyproject.toml), which is also where the proto bindings, the standard extension YAMLs and the ANTLR grammar come from — none of them are vendored here. The three are kept in lockstep by [`check_substrait_package_versions.sh`](check_substrait_package_versions.sh) (run in CI by [`version-checks.yml`](.github/workflows/version-checks.yml)), and the version they agree on is what `substrait.version.substrait_version` reports at runtime. + +Where the spec is genuinely unclear, don't settle it here. Survey the ecosystem for an existing consensus first. The closest comparison is the sibling language bindings listed under [Active Libraries](https://substrait.io/community/active_libraries/) — `substrait-go`, `substrait-java` and `substrait-rs` solve the same modeling problem at the same layer, so how they represent a construct is directly relevant; that page also separates active bindings from inactive ones, and an inactive binding's choice is weaker evidence. For questions about runtime semantics rather than modeling, the engines under [Powered by Substrait](https://substrait.io/community/powered_by/) (Acero, DataFusion, DuckDB, Gluten, Velox) are the better reference. + +If they agree, follow that de facto consensus and say so in the PR. If they disagree, or none of them cover the case, raise a clarification issue in [`substrait-io/substrait`](https://github.com/substrait-io/substrait/issues) or bring it to the [community](https://substrait.io/community/) channels rather than encoding a guess — and record the open question in the PR so the assumption stays reviewable. + +## Getting started + +### Get the repo + Fork and clone the repo. + ``` git clone https://github.com//substrait-python.git cd substrait-python ``` -## Development environment -Activate environment with uv. +### Development environment + +Create the environment with [uv](https://docs.astral.sh/uv/). + ``` -uv sync --extra test +uv sync ``` -# Lint & Format +That is enough to run the full test suite: the `dev` dependency group, which carries `pytest` along with the optional runtime dependencies the tests exercise (`substrait-antlr`, `pyyaml`, `sqloxide`, `deepdiff`, `duckdb`, `datafusion`), is synced by default. + +The two extras exist for consumers of the published package rather than for development, and can be added with `uv sync --extra `: + +* **`extensions`** — `substrait-antlr` and `pyyaml`, needed for the extension registry to resolve function overloads against the standard Substrait extensions. +* **`sql`** — `sqloxide` and `deepdiff`, needed by the `substrait.sql` front end. + +## Lint & format Run the following pixi tasks to lint and format with ruff. @@ -21,8 +54,48 @@ pixi run lint pixi run format ``` -# Test +Both are checked in CI, where the formatter runs as `pixi run format --check`. Use the pixi tasks rather than a separately installed ruff: the version is pinned under `[tool.pixi.dependencies]` in [`pyproject.toml`](pyproject.toml), and a different one may disagree about formatting. There is also a [`pre-commit`](https://pre-commit.com/) config wiring the same ruff lint and format hooks, if you prefer to have them run on commit. + +## Testing + Run tests in the project's root dir. + ``` uv run pytest ``` + +Narrow the run with a path or `-k` while iterating, e.g. `uv run pytest tests/dataframe -k lateral`. + +A clean run still reports skips. Those are the round-trips through external consumers in [`tests/sql`](tests/sql), skipped by default rather than for a missing dependency: those consumers lag the pinned spec, and handing them a plan built at a newer spec version can abort the interpreter natively. Opt in with `SUBSTRAIT_ENGINE_TESTS=1 uv run pytest` when you want them. + +The [examples](examples) run standalone, e.g. `uv run examples/builder_example.py`. CI runs four of them — `builder`, `duckdb`, `adbc`, `pyarrow` — on every PR ([`example.yml`](.github/workflows/example.yml)); `dataframe_example.py` and `narwhals_example.py` are outside that matrix, so changes to the DataFrame or Narwhals layers need those run by hand. Several examples are [PEP 723](https://peps.python.org/pep-0723/) scripts (a `# /// script` block declaring their own dependencies), which `uv run` executes in a separate environment built from the working tree rather than in your synced `.venv`. + +Tests run in CI ([`test.yml`](.github/workflows/test.yml)) as `uv run --frozen pytest` across Python 3.10–3.13 on Linux, macOS and Windows. Because of `--frozen`, a change to dependencies has to land together with a regenerated [`uv.lock`](uv.lock) — and [`pixi.lock`](pixi.lock) when it affects the lint environment — or CI will fail on the stale lock file rather than on your change. + +## Commit conventions + +Substrait Python follows [conventional commits](https://www.conventionalcommits.org/en/v1.0.0/) for commit message structure, and releases are automated from it (see [RELEASING.md](RELEASING.md)). Because pull requests are squash-merged, the message that ends up in history is built from the PR rather than from your local commits: please ensure that your PR title and description together form a valid commit message. The [PR Title Check](.github/workflows/pr_title.yml) workflow lints exactly that pair with commitlint and comments on the PR when it does not conform. + +Examples of commit messages can be seen [here](https://www.conventionalcommits.org/en/v1.0.0/#examples). + +## Pull requests + +Pull requests are squash-merged, and the **PR title and description become the commit message** that `semantic-release` parses to build [`CHANGELOG.md`](CHANGELOG.md) and the release notes. The title is the subject and the description is the body. [`.github/pull_request_template.md`](.github/pull_request_template.md) restates that where you write the description. + +Because the description is changelog input rather than a review scratchpad, leave out anything the diff and the CI checks already show: + +* **Lists of files touched** — they are in the diff. +* **Claims that CI-verified things pass** — "tests pass", "ruff clean". If they didn't, the checks would be red. +* **Process notes that are already implicit** — "opened as draft pending review". + +Do include the rationale, and for spec-tracking changes the spec version (e.g. `spec v0.99.0`). Keep the body free of git trailers (`Signed-off-by:`, `Co-authored-by:`) and tool-attribution lines; this project's history does not carry them. + +### Breaking changes + +Mark a breaking change twice: with `!` after the type and scope in the title (`feat!: …`), and with a `BREAKING CHANGE:` footer in the description. The `!` drives the version bump; the footer text is what populates the ⚠ BREAKING CHANGES section of the release notes, so describe what breaks and what consumers should do instead. Only a real footer counts — breaking-change prose under a `## Breaking change` heading is ordinary body text, and the note then degrades to a bare repeat of the subject line, which tells consumers nothing about how to migrate. + +Keep that footer **last, with nothing after it** — below the rationale and below any `Closes #NNN` line. The conventional-commits parser ends a `BREAKING CHANGE` note only at another footer keyword or an issue reference; anything else trailing it, whether prose, an attribution line, or a stray comment marker, is absorbed into the note and published verbatim. ([`.releaserc.mjs`](.releaserc.mjs) strips trailing git trailers such as `Signed-off-by:` for exactly this reason, but it matches only `Key: value` trailers, so it cannot recognize prose.) Putting the footer last also means the squash-merge message can be trimmed to just the subject and the footer in a single cut. + +Write the footer as unwrapped paragraphs. GitHub renders a single newline as a line break, so a hard-wrapped footer reaches the release notes broken mid-sentence. + +Note that this project is pre-1.0: a breaking change produces a **minor** bump, not a major one, matching the [Substrait versioning policy](https://substrait.io/spec/versioning/) and substrait-java. See [RELEASING.md](RELEASING.md) for the rest of the release process. diff --git a/README.md b/README.md index 524d688..775ed5b 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,13 @@ This project is not an execution engine for Substrait Plans. ## Status This is an experimental package that is still under development. +## Substrait version +The specification version a release targets is set by the `substrait-protobuf`, +`substrait-extensions` and `substrait-antlr` requirements in +[`pyproject.toml`](pyproject.toml) — the proto bindings, the standard extension +definitions and the grammar all come from those packages rather than being vendored +here. `substrait.version.substrait_version` reports it at runtime. + # Building plans with the DataFrame API The `substrait.dataframe` module is an ergonomic, fluent API for authoring @@ -120,7 +127,7 @@ relations { names: "first_name" } } ->>> serialized_plan = p.SerializeToString() +>>> serialized_plan = plan.SerializeToString() >>> serialized_plan b'\x1aA\x12?\n1\n/\x12#\n\nfirst_name\n\x07surname\x12\x0c\n\x04b\x02\x10\x02\n\x04b\x02\x10\x02:\x08\n\x06people\x12\nfirst_name' ``` @@ -169,7 +176,6 @@ relations { ## Load a Substrait Plan from JSON A substrait plan can be loaded [from the JSON representation of the protobuf message](https://protobuf.dev/programming-guides/json/) using the [`protobuf` python library](https://pypi.org/project/protobuf/): -functions: ``` >>> from substrait.proto import Plan @@ -214,7 +220,7 @@ functions: ... } ... ] ... }""" ->>> json_format.Parse(text=jsontext, mesage=Plan()) +>>> json_format.Parse(text=jsontext, message=Plan()) relations { root { input { @@ -391,3 +397,15 @@ version { producer: "ibis-substrait" } ``` + +# Contributing + +See [CONTRIBUTING.md](CONTRIBUTING.md) for how the specification relates to this +repository, the development environment, the lint / format / test commands, and the +commit and pull request conventions. Releases are automated — [RELEASING.md](RELEASING.md) +describes that process. + +# Getting Involved + +To learn more, head over to [Substrait](https://substrait.io/), our parent project, and +join our [community](https://substrait.io/community/).