-
Notifications
You must be signed in to change notification settings - Fork 784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A new example that shows how to integrate Python plugins that use pyclasses into a Rust app #2873
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I have some thoughts:
This PR uses the word "plugin" a lot. What does it mean? If you mean "extension module", you should call it that, not "plugin". We don't call it that anywhere, so this is very likely to confuse readers, and it confused me too until I realized you meant extension module (I think).
I'm also not sure this is the correct approach. Personally I'd update the existing examples - just put this at the bottom of their lib.rs:
#[cfg(test)]
mod tests{
#[test]
fn test_module(){
// make module
// call module etc
}
}
This could be added to maturin new
too.
There are two advantages to this:
- When users create their first python module, they won't have to figure out how to test it. It will just be there
- Putting it in lib.rs (or anywhere else in the crate) allows users to access crate private things.
main.rs
does not let you do this.
What do you think?
examples/plugin/src/main.rs
Outdated
pyo3::append_to_inittab!(pylib_module); | ||
//spawn runtime | ||
pyo3::prepare_freethreaded_python(); | ||
//import path for python | ||
let path = Path::new("./"); | ||
//do useful work | ||
Python::with_gil(|py| { | ||
//add the current directory to import path of Python (do not use this in production!) | ||
let syspath: &PyList = py.import("sys")?.getattr("path")?.extract()?; | ||
syspath.insert(0, &path)?; | ||
println!("Import path is: {:?}", syspath); | ||
|
||
// Now we can load our plugin.py file | ||
let plugin = PyModule::import(py, "plugin")?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just call the initialization function, and avoid messing with sys.path entirely:
let module = PyModule::new(py, "plugin")?;
plugin_api::plugin_api(py, &module)?;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this will not work as expected, since plugin.py will not land on the import path. And then, if plugin.py has any dependencies of its own, they will fail to import which will confuse the user until they figure out they need to mess with import path for it to work as you'd expect Python to work. I will actually add some deps to plugin.py to illustrate this.
This will only work with custom build configuration (as in my build_extmod.sh) to make the actual plugin. With the way linking is set up with Python you can not have both tests and extension modules working at the same time. Though it is a "larger issue" in some sense. |
Depending on what you guys decide on w.r.t. the plug-ins-versus-better-testing angle, I think it should be stressed that while putting the plug-in API in a separate crate and testing plug-ins this way is possible, it is not possible to build the plug-in API/host separately from the main application, i.e. it currently needs to be linked statically into the main application to keep their ABI compatible. |
That is very much true, and is a fundamental limitation of Rust toolchain at the moment. Not sure if such remarks belong in the example though, but feel free to suggest where they would go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for not getting back to you sooner 😅. I have some comments remaining.
Anyway, I wonder what the others think E.g. @davidhewitt ?
No rush here=) Thanks for the suggestions, I think your description is quite well fitting, and indeed way shorter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, this looks excellent to me, thanks very much to both of you for putting this example together!
I'll add the template files and merge when I get a moment 👍
…to a Rust app while having option to test API without the main app
bors r+ |
2873: A new example that shows how to integrate Python plugins that use pyclasses into a Rust app r=davidhewitt a=alexpyattaev Example showing integration of a Python plugin into a Rust app while having option to test pyclass based API without the main app. This also illustrates some aspects related to import of Python modules into a Rust app while also having an API module available for the Python code to be able to produce Rust objects. CI seems to fail on my local machine for reasons unrelated to the example just added: ``` error: unused macro definition: `check_struct` --> pyo3-ffi-check/src/main.rs:13:18 | 13 | macro_rules! check_struct { | ^^^^^^^^^^^^ | ``` 2889: Added support for PyErr_WriteUnraisable r=davidhewitt a=mitsuhiko Fixes #2884 2923: hygiene: fix `#[pymethods(crate = "...")]` r=davidhewitt a=davidhewitt Got to the bottom of the hygiene issue in test of #2914 Turns out that `#[pymethods] #[pyo3(crate = "...")]` works, but `#[pymethods(crate = "...")]` was ignoring the argument. Added a tweak to fix this and a snippet in the hygiene test (which fails on `main`). 2924: remove unneeded into_iter calls r=davidhewitt a=davidhewitt Clippy complaining about these to me this morning locally. Co-authored-by: Alex Pyattaev <alex.pyattaev@gmail.com> Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Co-authored-by: Armin Ronacher <armin.ronacher@active-4.com>
Build failed (retrying...): |
2873: A new example that shows how to integrate Python plugins that use pyclasses into a Rust app r=davidhewitt a=alexpyattaev Example showing integration of a Python plugin into a Rust app while having option to test pyclass based API without the main app. This also illustrates some aspects related to import of Python modules into a Rust app while also having an API module available for the Python code to be able to produce Rust objects. CI seems to fail on my local machine for reasons unrelated to the example just added: ``` error: unused macro definition: `check_struct` --> pyo3-ffi-check/src/main.rs:13:18 | 13 | macro_rules! check_struct { | ^^^^^^^^^^^^ | ``` 2889: Added support for PyErr_WriteUnraisable r=davidhewitt a=mitsuhiko Fixes #2884 Co-authored-by: Alex Pyattaev <alex.pyattaev@gmail.com> Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Co-authored-by: Armin Ronacher <armin.ronacher@active-4.com>
Build failed (retrying...): |
2873: A new example that shows how to integrate Python plugins that use pyclasses into a Rust app r=davidhewitt a=alexpyattaev Example showing integration of a Python plugin into a Rust app while having option to test pyclass based API without the main app. This also illustrates some aspects related to import of Python modules into a Rust app while also having an API module available for the Python code to be able to produce Rust objects. CI seems to fail on my local machine for reasons unrelated to the example just added: ``` error: unused macro definition: `check_struct` --> pyo3-ffi-check/src/main.rs:13:18 | 13 | macro_rules! check_struct { | ^^^^^^^^^^^^ | ``` Co-authored-by: Alex Pyattaev <alex.pyattaev@gmail.com> Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
Build failed: |
Not sure what bors is unhappy with... Any ideas? There seems to be some issue fetching python 3.11 on Darwin from "https://github.com/actions/python-versions/releases/download/3.11.0-3730290910/python-3.11.0-darwin-x64.tar.gz", is that it? |
bors retry |
There's a compatibility issue with our test suite on 3.11.1 (#2817) so we're using 3.11.0 for testing at the moment (3.11.2 should fix this). I think we just hit a rate limit with all the bors tests running! |
Build succeeded: |
Example showing integration of a Python plugin into a Rust app while having option to test pyclass based API without the main app. This also illustrates some aspects related to import of Python modules into a Rust app while also having an API module available for the Python code to be able to produce Rust objects.
CI seems to fail on my local machine for reasons unrelated to the example just added: