apache / apache/datafusion-python

Gate pyo3/extension-module so the crate can be linked as a Rust dependency and tested

オープン
#1,703 コメント 1 件 リアクション 0 件 担当者 1 名 @emecii が担当を希望しています GitHub で見る
development-process enhancement rust
主要言語
Python
スター
604
フォーク
174
平均マージ
1日 7時間
マージ済み PR(30日)
4

説明

**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**

`crates/core/Cargo.toml` enables `pyo3/extension-module` unconditionally. That feature tells pyo3 not to link against `libpython`, which is right for the extension module the wheel ships, but it makes the crate unusable in any build that needs to link `Py_*` symbols itself. Two things are blocked by this today.

The first is Rust tests. No workflow invokes `cargo test`; the only Rust checks in CI are `cargo fmt --check` and `cargo clippy --no-deps --all-targets`. `--all-targets` compiles `#[cfg(test)]` code, so a Rust test cannot rot into a non-compiling state, but it is never executed and a behavioral regression will not fail the build. Adding the job is not a one-line change, because the test binary fails to link on Linux for exactly the reason above. This is recorded in `AGENTS.md` as the reason Rust tests are currently dead weight in this repository.

The second is consuming `datafusion-python` as an ordinary Rust dependency. `crate-type` already includes `rlib`, so this looks supported, but a downstream crate that builds a binary hits the same link failure. This came up in https://github.com/apache/datafusion-python/pull/1678#pullrequestreview-5100366976, where the request was to make some of the Python UDF serialization internals public so they could be plugged into an existing physical codec in a distributed setup. Marking those items `pub` would advertise an API that a downstream crate cannot actually link against, so the visibility change is not the useful part on its own.

**Describe the solution you'd like**

Put `pyo3/extension-module` behind a Cargo feature that is on by default (so the wheel build and `maturin develop` are unchanged) and can be turned off by a consumer or a test build. Then add a `cargo test` job to CI in the same change, so the tests that exist actually run and the gate does not silently regress.

**Describe alternatives you've considered**

Leaving it as is and keeping all Rust behavior covered from Python. That is the current practice and it works well for the user-facing surface, which is the primary focus anyway. It does not help the downstream-consumer case, and it means genuinely Rust-only invariants have no executable coverage.

Splitting the crate, with the reusable pieces in a library crate that does not depend on `extension-module` and the pyo3 bindings in a thin crate on top. Cleaner in the long run and a much larger change; worth considering if the feature gate turns out to be awkward.

**Additional context**

Follow-up from #1678. This one is a prerequisite for exposing any Rust-facing API from this crate, including the codec work requested in that review.

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。