rust-numpy icon indicating copy to clipboard operation
rust-numpy copied to clipboard

Opt-in feature to enable pyo3/multiple-pymethods feature

Open jfortin42 opened this issue 1 year ago • 5 comments

Hi there,

Context

We’ve encountered an issue while using the rust-numpy crate in conjunction with the pyo3 multiple-pymethods feature in one of our internal projects. This feature is essential for implementing methods on a structure across multiple files, which helps in organizing larger codebases. However, since rust-numpy does not currently enable this feature, it caused conflicts when trying to use both crates together.

Reproducible example

reproducible_example.zip

What this pull request does

Simply adds a opt-in multiple-pymethods feature to enable the pyo3/multiple-pymethods feature.

We believe this change will improve the usability of rust-numpy for users with similar requirements Thank you for considering this PR!

Best regards, Nuant Team

jfortin42 avatar Dec 06 '24 14:12 jfortin42

This would cause the numpy crate to no longer compile for pyodide (target wasm32-unknown-emscripten). https://github.com/PyO3/pyo3/issues/2517

I don't think it should be necessary for numpy to enable this feature for you to be able to use it in your own project.

kylebarron avatar Dec 06 '24 15:12 kylebarron

This would cause the numpy crate to no longer compile for pyodide (target wasm32-unknown-emscripten). PyO3/pyo3#2517

I don't think it should be necessary for numpy to enable this feature for you to be able to use it in your own project.

The idea is to offer an opt-in feature to enable pyo3's multiple-pymethods feature in the numpy crate (as is already the case for the pyo3/gil-refs feature)

jfortin42 avatar Dec 06 '24 15:12 jfortin42

There is no need to do that. rust-numpy has no dependency on that feature and cargo will do feature unification behind the scenes. So if you depend on pyo3 with that feature enabled, cargo will compile rust-numpy agains it as well.

With a simple starter project I have no problem compiling with the following Cargo.toml

pyo3 = { version = "0.22", features = ["multiple-pymethods"] }
numpy = "0.22"

(gil-refs was different, because rust-numpy needed to conditionally compile itself as well.)

Icxolu avatar Dec 06 '24 18:12 Icxolu

There is no need to do that. rust-numpy has no dependency on that feature and cargo will do feature unification behind the scenes. So if you depend on pyo3 with that feature enabled, cargo will compile rust-numpy agains it as well.

This is not the case with proc-macros, which is our use case:

Build-dependencies and proc-macros do not share features with normal dependencies.

source: https://doc.rust-lang.org/cargo/reference/features.html#feature-resolver-version-2

With a simple starter project I have no problem compiling with the following Cargo.toml

You can take a look at the simple example we've provided which highlights this problem.

jfortin42 avatar Dec 06 '24 19:12 jfortin42

I can't really tell whos fault this is. From my perpective these should be completely independend builds that should interfere with each other. My guess would be that this is some weird inventory sideeffect.

You can simply add pyo3 with that feature enabled along side rust-numpy and take advantage feature-unification for the proc-macro build by itself to make it compile, but whether this actually will work as intended and not unpredictably share some code between the proc-macro and the lib I can't tell.

Anyway I still don't think adding a feature makes sense, it's always possible to also add pyo3 directly as well and enable it that way.

Icxolu avatar Dec 06 '24 20:12 Icxolu