Skip to content
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

make GILOnceCell::get_or_try_init_type_ref public #4542

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions guide/src/python-from-rust/calling-existing-code.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,17 @@

If you already have some existing Python code that you need to execute from Rust, the following FAQs can help you select the right PyO3 functionality for your situation:

## Want to access Python APIs? Then use `PyModule::import`.
## Want to access Python APIs? Then use `import`.

[`PyModule::import`]({{#PYO3_DOCS_URL}}/pyo3/types/struct.PyModule.html#method.import) can
be used to get handle to a Python module from Rust. You can use this to import and use any Python
[`Python::import`] can be used to get handle to a Python module from Rust. You can use this to import and use any Python
module available in your environment.

```rust
use pyo3::prelude::*;

fn main() -> PyResult<()> {
Python::with_gil(|py| {
let builtins = PyModule::import(py, "builtins")?;
let builtins = py.import("builtins")?;
let total: i32 = builtins
.getattr("sum")?
.call1((vec![1, 2, 3],))?
Expand All @@ -24,9 +23,16 @@ fn main() -> PyResult<()> {
}
```

## Want to run just an expression? Then use `eval_bound`.
[`Python::import`] introduces an overhead each time it's called. To avoid this in functions that are called multiple times,
using a [`GILOnceCell`]({{#PYO3_DOCS_URL}}/pyo3/sync/struct.GILOnceCell.html) is recommended. Specifically, for importing types,
[`GILOnceCell::get_or_try_init_type_ref`]({{#PYO3_DOCS_URL}}/pyo3/sync/struct.GILOnceCell.html#method.get_or_try_init_type_ref) is provided
(check out the [example]({{#PYO3_DOCS_URL}}/pyo3/sync/struct.GILOnceCell.html#example-using-giloncecell-to-avoid-the-overhead-of-importing-a-class-multiple-times)).

[`Python::eval_bound`]({{#PYO3_DOCS_URL}}/pyo3/marker/struct.Python.html#method.eval_bound) is
[`Python::import`]: {{#PYO3_DOCS_URL}}/pyo3/marker/struct.Python.html#method.import

## Want to run just an expression? Then use `eval`.

[`Python::eval`]({{#PYO3_DOCS_URL}}/pyo3/marker/struct.Python.html#method.eval) is
a method to execute a [Python expression](https://docs.python.org/3/reference/expressions.html)
and return the evaluated value as a `Bound<'py, PyAny>` object.

Expand All @@ -48,17 +54,19 @@ Python::with_gil(|py| {
# }
```

## Want to run statements? Then use `run_bound`.
## Want to run statements? Then use `run`.

[`Python::run_bound`] is a method to execute one or more
[`Python::run`] is a method to execute one or more
[Python statements](https://docs.python.org/3/reference/simple_stmts.html).
This method returns nothing (like any Python statement), but you can get
access to manipulated objects via the `locals` dict.

You can also use the [`py_run!`] macro, which is a shorthand for [`Python::run_bound`].
You can also use the [`py_run!`] macro, which is a shorthand for [`Python::run`].
Since [`py_run!`] panics on exceptions, we recommend you use this macro only for
quickly testing your Python extensions.

[`Python::run`]: {{#PYO3_DOCS_URL}}/pyo3/marker/struct.Python.html#method.run

```rust
use pyo3::prelude::*;
use pyo3::py_run;
Expand Down
1 change: 1 addition & 0 deletions newsfragments/4542.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Change `GILOnceCell::get_or_try_init_type_ref` to public.
30 changes: 29 additions & 1 deletion src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,35 @@ impl GILOnceCell<Py<PyType>> {
/// Get a reference to the contained Python type, initializing it if needed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify that "it" refers to the GILOnceCell, not the class in it

Suggested change
/// Get a reference to the contained Python type, initializing it if needed.
/// Get a reference to the contained Python type, initializing the cell if needed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! bcb883c

///
/// This is a shorthand method for `get_or_init` which imports the type from Python on init.
pub(crate) fn get_or_try_init_type_ref<'py>(
///
/// # Example: Using `GILOnceCell` to avoid the overhead of importing a class multiple times
///
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better to just say what the example does, this makes for nicer URLs as well.

Suggested change
/// # Example: Using `GILOnceCell` to avoid the overhead of importing a class multiple times
///
/// # Example: Using `GILOnceCell` to store a class in a static variable.
///
/// `GILOnceCell` can be used to avoid importing a class multiple times

(this also explicitly mentions "static", which is important. We don't want people to have local (or worse, const) GILOnceCells).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: 5d95fba

/// ```
/// # use pyo3::prelude::*;
/// # use pyo3::sync::GILOnceCell;
/// # use pyo3::types::{PyDict, PyType};
/// # use pyo3::intern;
/// #
/// #[pyfunction]
/// fn create_ordered_dict<'py>(py: Python<'py>, dict: Bound<'py, PyDict>) -> PyResult<Bound<'py, PyAny>> {
/// // Even if this function is called multiple times,
/// // the `OrderedDict` class will be imported only once.
/// static ORDERED_DICT: GILOnceCell<Py<PyType>> = GILOnceCell::new();
/// ORDERED_DICT
/// .get_or_try_init_type_ref(py, "collections", "OrderedDict")?
/// .call1((dict,))
/// }
///
/// # Python::with_gil(|py| {
/// # let dict = PyDict::new(py);
/// # dict.set_item(intern!(py, "foo"), 42).unwrap();
/// # let fun = wrap_pyfunction!(create_ordered_dict, py).unwrap();
/// # let ordered_dict = fun.call1((&dict,)).unwrap();
/// # assert!(dict.eq(ordered_dict).unwrap());
/// # });
/// ```
///
pub fn get_or_try_init_type_ref<'py>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can use a better name. Maybe just "import"? 🤔

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but then maybe it should be implemented for GILOnceCell<Py<T>> where T: PyTypeInfo instead of GILOnceCell<Py<PyType>>? Otherwise, I would call it import_type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be fine with that. I think you want T: PyTypeCheck rather than T: PyTypeInfo, though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, done in 79568f6!

&self,
py: Python<'py>,
module_name: &str,
Expand Down
Loading