-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Embedded module with an id example #6061
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA new Rust example file demonstrates how to inject a custom Python module into the RustPython interpreter and access it from a Rust-defined global function callable from Python. The example shows state sharing between Rust and Python by registering a module with an attribute and exposing a Rust function to Python code. Changes
Sequence Diagram(s)sequenceDiagram
participant Rust Main
participant RustPython VM
participant Python Code
Rust Main->>RustPython VM: Initialize VM without stdlib
Rust Main->>RustPython VM: Register get_id as global Python function
Rust Main->>RustPython VM: Create Python module (__id_module) with __id=42
Rust Main->>RustPython VM: Inject module into sys.modules
Rust Main->>RustPython VM: Execute Python code: get_id()
Python Code->>RustPython VM: Call get_id()
RustPython VM->>Rust Main: Invoke get_id (Rust function)
Rust Main->>RustPython VM: Import __id_module, access __id, print and return value
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 4
🧹 Nitpick comments (1)
examples/embedded_id.rs (1)
19-22
: Consider using RustPython's macro system for function registration.While the manual function registration works, RustPython provides macros that can simplify this process. However, for an embedding example showing the low-level API, this approach is educational.
If you want to demonstrate the macro approach, you could use:
+use rustpython_vm::{pyfunction, PyResult}; + +#[pyfunction] +fn print_id(vm: &vm::VirtualMachine) -> PyResult<()> { + // function implementation +}Then register it more simply. But the current approach is fine for demonstrating the embedding API.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/embedded_id.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.rs
: Follow the default rustfmt code style (cargo fmt
to format)
Always run clippy to lint code (cargo clippy
) before completing tasks. Fix any warnings or lints that are introduced by your changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass
,pymodule
,pyfunction
, etc.) when implementing Python functionality in Rust
Files:
examples/embedded_id.rs
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: In most cases, Python code should not be edited. Bug fixes should be made through Rust code modifications only
Learnt from: moreal
PR: RustPython/RustPython#5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
Learnt from: moreal
PR: RustPython/RustPython#5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.
📚 Learning: applies to **/*.rs : use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implement...
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
Applied to files:
examples/embedded_id.rs
🔇 Additional comments (3)
examples/embedded_id.rs (3)
1-3
: LGTM: Import statements are appropriate for the embedding example.The imports correctly bring in the necessary RustPython components for embedding. The wildcard import is acceptable for an example, though in production code you might prefer more specific imports to avoid namespace pollution.
15-16
: LGTM: Proper interpreter initialization.The interpreter setup without stdlib is appropriate for this focused example, and creating a scope with builtins provides the necessary Python environment.
41-46
: LGTM: Proper code compilation and execution with error handling.The code compilation and execution demonstrates proper error handling with the
?
operator and appropriate error conversion from compilation errors to Python syntax errors.
examples/embedded_id.rs
Outdated
fn main() -> vm::PyResult<()> { | ||
vm::Interpreter::without_stdlib(Default::default()).enter(|vm| { | ||
let scope = vm.new_scope_with_builtins(); | ||
|
||
// Register the global function | ||
let _ = | ||
scope | ||
.globals | ||
.set_item("print_id", vm.new_function("print_id", print_id).into(), vm); | ||
|
||
// Create a module and set an id. | ||
let module = PyModule::new().into_ref(&vm.ctx); | ||
module | ||
.as_object() | ||
.set_attr("__id", vm.new_pyobj(42_i32), vm) | ||
.ok() | ||
.unwrap(); | ||
|
||
// Register the module | ||
let sys = vm.import("sys", 0).ok().unwrap(); | ||
let modules = sys.get_attr("modules", vm).ok().unwrap(); | ||
modules | ||
.set_item("__id_module", module.into(), vm) | ||
.ok() | ||
.unwrap(); | ||
|
||
// Execute the code | ||
let source = r#"print_id()"#; | ||
let code_obj = vm | ||
.compile(source, vm::compiler::Mode::Exec, "<embedded>".to_owned()) | ||
.map_err(|err| vm.new_syntax_error(&err, Some(source)))?; | ||
|
||
vm.run_code_obj(code_obj, scope)?; | ||
|
||
Ok(()) | ||
}) | ||
} |
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.
💡 Verification agent
❓ Verification inconclusive
Valuable embedding example with room for error handling improvements.
This example effectively demonstrates the key concepts for embedding RustPython and creating modules with attributes. The workflow is clear and educational. The main areas for improvement are:
- Consistent error handling throughout (mixing
?
and.unwrap()
) - Consider demonstrating both low-level and macro-based approaches
Please run cargo clippy
on this example to ensure it follows Rust best practices:
🏁 Script executed:
#!/bin/bash
# Check for clippy warnings on the new example
cargo clippy --bin embedded_id
Length of output: 142
Enforce Consistent Error Handling & Showcase Macro-Based APIs
The example clearly illustrates embedding RustPython, but it mixes panics (unwrap()
) with ?
propagation. To improve reliability and teach best practices:
• Refactor .ok().unwrap()
calls in examples/embedded_id.rs to use ?
or .expect("failed to …")
with a clear message:
– module.as_object().set_attr(…)
– vm.import("sys", …).ok()
– sys.get_attr("modules", …).ok()
• Maintain the existing ?
-based error handling for compile()
and run_code_obj()
.
• (Optional) Add a second variant of this example using RustPython’s macros—e.g.:
#[pyfunction]
fn print_id(vm: &VirtualMachine, …) -> PyResult<…> { … }
#[pymodule]
fn embedded_id(vm: &VirtualMachine) -> PyResult<()> { … }
to demonstrate the more ergonomic, macro-driven approach.
Please manually run cargo clippy --bin embedded_id
on your machine to catch any remaining lint or style issues.
🤖 Prompt for AI Agents
In examples/embedded_id.rs from lines 14 to 50, the code inconsistently handles
errors by mixing unwrap() calls with ? propagation. Refactor all .ok().unwrap()
calls to use ? for error propagation or .expect() with clear, descriptive
messages to improve reliability and clarity. Keep the existing ? usage for
compile() and run_code_obj(). Optionally, add a second example variant using
RustPython’s #[pyfunction] and #[pymodule] macros to demonstrate a more
ergonomic macro-based API. Finally, run cargo clippy --bin embedded_id locally
to ensure all lint and style issues are addressed.
examples/embedded_id.rs
Outdated
let module = PyModule::new().into_ref(&vm.ctx); | ||
module | ||
.as_object() | ||
.set_attr("__id", vm.new_pyobj(42_i32), vm) | ||
.ok() | ||
.unwrap(); |
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.
🛠️ Refactor suggestion
Inconsistent error handling pattern.
You're using .ok().unwrap()
here while using ?
operator elsewhere in the function. Consider using consistent error handling throughout.
Apply this diff for consistent error handling:
let module = PyModule::new().into_ref(&vm.ctx);
- module
- .as_object()
- .set_attr("__id", vm.new_pyobj(42_i32), vm)
- .ok()
- .unwrap();
+ module
+ .as_object()
+ .set_attr("__id", vm.new_pyobj(42_i32), vm)?;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
let module = PyModule::new().into_ref(&vm.ctx); | |
module | |
.as_object() | |
.set_attr("__id", vm.new_pyobj(42_i32), vm) | |
.ok() | |
.unwrap(); | |
let module = PyModule::new().into_ref(&vm.ctx); | |
module | |
.as_object() | |
.set_attr("__id", vm.new_pyobj(42_i32), vm)?; |
🤖 Prompt for AI Agents
In examples/embedded_id.rs around lines 25 to 30, the code uses .ok().unwrap()
for error handling, which is inconsistent with the rest of the function that
uses the ? operator. Replace .ok().unwrap() with the ? operator to propagate
errors consistently and cleanly.
examples/embedded_id.rs
Outdated
let sys = vm.import("sys", 0).ok().unwrap(); | ||
let modules = sys.get_attr("modules", vm).ok().unwrap(); | ||
modules | ||
.set_item("__id_module", module.into(), vm) | ||
.ok() | ||
.unwrap(); |
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.
🛠️ Refactor suggestion
Inconsistent error handling pattern (repeated issue).
Same issue as above - mixing .ok().unwrap()
with ?
operator usage.
Apply this diff for consistent error handling:
- let sys = vm.import("sys", 0).ok().unwrap();
- let modules = sys.get_attr("modules", vm).ok().unwrap();
- modules
- .set_item("__id_module", module.into(), vm)
- .ok()
- .unwrap();
+ let sys = vm.import("sys", 0)?;
+ let modules = sys.get_attr("modules", vm)?;
+ modules
+ .set_item("__id_module", module.into(), vm)?;
🤖 Prompt for AI Agents
In examples/embedded_id.rs around lines 33 to 38, the code uses a mix of
.ok().unwrap() for error handling, which is inconsistent with the rest of the
code that uses the ? operator. Replace all instances of .ok().unwrap() with the
? operator to propagate errors properly and maintain consistent error handling
style throughout the function.
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.
Thank you so much! The example looks great.
Please check CI failure and the comment about file name/header.
examples/embedded_id.rs
Outdated
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.
How about examples/module_injection.rs
?
The key technique of this example is creating a module and putting it into sys.module.
Please also add what __id_module
is and how it can be useful to the top of the file.
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.
I think I addressed all issues ?
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
examples/module_injection.rs (1)
23-25
: Consider handling the registration result.While the function registration is correct, ignoring the result with
let _ =
could mask potential errors.Consider handling the result:
- let _ = scope - .globals - .set_item("get_id", vm.new_function("get_id", get_id).into(), vm); + scope + .globals + .set_item("get_id", vm.new_function("get_id", get_id).into(), vm)?;
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/module_injection.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.rs
: Follow the default rustfmt code style (cargo fmt
to format)
Always run clippy to lint code (cargo clippy
) before completing tasks. Fix any warnings or lints that are introduced by your changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass
,pymodule
,pyfunction
, etc.) when implementing Python functionality in Rust
Files:
examples/module_injection.rs
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: In most cases, Python code should not be edited. Bug fixes should be made through Rust code modifications only
Learnt from: moreal
PR: RustPython/RustPython#5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
Learnt from: moreal
PR: RustPython/RustPython#5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.
📚 Learning: applies to **/*.rs : use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implement...
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
Applied to files:
examples/module_injection.rs
🔇 Additional comments (5)
examples/module_injection.rs (5)
8-16
: LGTM! Clear demonstration of module attribute access.The function correctly demonstrates how to:
- Import an injected module using
vm.import
- Extract attributes with proper type conversion
- Handle errors appropriately with the
?
operatorThe implementation follows RustPython patterns well for this example scenario.
19-20
: Good choice for example scope.Using
without_stdlib
is appropriate for this focused example, keeping it lightweight while still providing necessary builtins.
28-36
: Excellent demonstration of module injection pattern.This section clearly shows the core concept:
- Creating a module with
PyModule::new()
- Setting custom attributes on the module
- Properly injecting it into
sys.modules
The implementation follows RustPython APIs correctly and demonstrates the intended functionality well.
39-44
: Proper code compilation and execution.The code compilation and execution follow RustPython best practices:
- Correct use of
Mode::Exec
for the script- Proper error handling with
map_err
for compilation errors- Good debugging support with meaningful filename
5-7
: Excellent documentation of the use case.The comment clearly explains the practical application for multi-threaded environments and provides concrete suggestions for extending the pattern with
Arc<Mutex<HashMap>>
.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
I had a bit of a tough time getting this to work API wise so I thought maybe a small example would be helpful.
It creates a module, sets an id attribute to the module, registers the module and calls a global fn which reads out the module and id.
Maybe it is useful.
Summary by CodeRabbit