timsaucer opened a new issue, #1703:
URL: https://github.com/apache/datafusion-python/issues/1703

   **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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to