docs: major rewrite for Bound API by davidhewitt · Pull Request #3906 · PyO3/pyo3

davidhewitt · 2024-02-26T23:24:04Z

I started these on the weekend but ran out of time to go any further, I'll try to continue with these in the next couple of days...

guide/src/python_from_rust/calling-existing-code.md

guide/src/python_from_rust.md

codspeed-hq · 2024-03-08T12:21:24Z

CodSpeed Performance Report

Merging #3906 will not alter performance

_{Comparing davidhewitt:bound-docs (0547c63) with main (14d1d2a)}

🎉 Hooray! `pytest-codspeed` just leveled up to 2.2.0!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

✅ 67 untouched benchmarks

Icxolu · 2024-03-08T22:09:20Z

guide/src/python_from_rust.md


-* [`call`]({{#PYO3_DOCS_URL}}/pyo3/prelude/trait.PyAnyMethods#tymethod.call) - call any callable Python object.
-* [`call_method`]({{#PYO3_DOCS_URL}}/pyo3/prelude/trait.PyAnyMethods#tymethod.call_method) - call a method on the Python object.
+* It provides global APIs for the Python interpreter, such as [`py.eval()`][eval] and [`py.import()`][import].


Suggested change

* It provides global APIs for the Python interpreter, such as [`py.eval()`][eval] and [`py.import()`][import].

* It provides global APIs for the Python interpreter, such as [`py.eval_bound()`][eval] and [`py.import_bound()`][import].

guide/src/python_from_rust.md

Icxolu · 2024-03-08T22:30:57Z

guide/src/types.md

+PyO3's API offers three generic smart pointers: `Py<T>`, `Bound<'py, T>` and `Borrowed<'a, 'py, T>`. For each of these the type parameter `T` will be filled by a [concrete Python type](#concrete-python-types). For example, a Python list object can be represented by `Py<PyList>`, `Bound<'py, PyList>`, and `Borrowed<'a, 'py, PyList>`.

-## The Python GIL, mutability, and Rust types
+These smart pointers have different numbers of lifetime parameters, which defines how they behave. `Py<T>` has no lifetime parameters, `Bound<'py, T>` has a lifetime parameter `'py`, and `Borrowed<'a, 'py, T>` has two lifetime parameters `'a` and `'py`.


If we make a point here about these, we should also explain what these lifetimes describe, not just that they are there.
What does 'py describe? Is it the same for Bound and Borrowed? What about 'a?

Or we remove this here and defer to the detail sections below.

Tried to expand on this a bit.

guide/src/types.md

guide/src/python_from_rust/calling-existing-code.md

MarshalX · 2024-03-10T00:29:02Z

guide/src/types.md

+
+By having the binding to the `'py` lifetime, `Bound<'py, T>` can offer the complete PyO3 API at maximum efficiency. This means that in almost all cases where `Py<T>` is not necessary for lifetime reasons, `Bound<'py, T>` should be used.
+
+`Bound<'py, T>` engages in Python reference counting. This means that `Bound<'py, T>` owns a Python object. Rust code which just wants to borrow a Python object should use a shared reference `&Bound<'py, T>`. Just like `std::sync::Arg`, using `.clone()` and `drop()` will cheaply implement and decrement the reference count of the object (just in this case, the reference counting is implemented by the Python interpreter itself).


Suggested change

`Bound<'py, T>` engages in Python reference counting. This means that `Bound<'py, T>` owns a Python object. Rust code which just wants to borrow a Python object should use a shared reference `&Bound<'py, T>`. Just like `std::sync::Arg`, using `.clone()` and `drop()` will cheaply implement and decrement the reference count of the object (just in this case, the reference counting is implemented by the Python interpreter itself).

`Bound<'py, T>` engages in Python reference counting. This means that `Bound<'py, T>` owns a Python object. Rust code which just wants to borrow a Python object should use a shared reference `&Bound<'py, T>`. Just like `std::sync::Arg`, using `.clone()` and `drop()` will cheaply increment and decrement the reference count of the object (just in this case, the reference counting is implemented by the Python interpreter itself).

Also, drive by comment: Arg should be Arc

MarshalX · 2024-03-10T00:44:19Z

I feel the lack of understanding how #[pyfunction] works. What should I specify as return type. Same with args. I believe that there is a hidden type conversation behind the scene. So I'm confused should I return Bound<> or PyObject... which will not cause type conversation. Should I use PyResult<&str> or return PyResult of Bound PyString. Should I accept args as refs of not. Unfortunately, most code snippets utilize PyResult<()> as the return type :( I will be happy to find answers to these questions in the docs

Speaking of v0.20 I was surprised to gain a huge performance bust by
changing argument type:

-fn _decode_dag_cbor(data: Vec<u8>) -> Result<Ipld> {
+fn _decode_dag_cbor(data: &[u8]) -> Result<Ipld> {

I wish to not make this mistake again

Co-authored-by: Lily Foote <code@lilyf.org>

davidhewitt · 2024-03-10T12:33:41Z

Thanks, I've added a new section specifically talking about the function argument lifetimes to help indicate to use Bound (and why Py is sometimes ok too).

I'd like Vec<u8> to keep to separate issues like #2888, there is definitely discussion to have there.

Icxolu · 2024-03-10T12:49:49Z

guide/src/conversions/traits.md

    - apply a custom function to convert the field from Python the desired Rust type.
    - the argument must be the name of the function as a string.
-    - the function signature must be `fn(&PyAny) -> PyResult<T>` where `T` is the Rust type of the argument.
+    - the function signature must be `fn(Bound<PyAny>) -> PyResult<T>` where `T` is the Rust type of the argument.


This can also take a &Bound<PyAny> I believe

I think it is actually just &Bound<PyAny> for now.

Icxolu · 2024-03-10T12:53:48Z

guide/src/memory.md

If we want to keep this section referencing the gil-refs for now, we probably should backport the the examples again, otherwise this does not make a lot of sense

Good idea 👍

guide/src/performance.md

src/lib.rs

guide/src/types.md

Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>

guide/src/python-from-rust.md

Co-authored-by: Adam Reichold <adamreichold@users.noreply.github.com>

Icxolu

Looks good 🚀 ! There is still calling-existing-code.md to rework, but I think it's fine to merge this as is for the beta and then finish the docs towards final release.

guide/src/async-await.md

Co-authored-by: Adam Reichold <adamreichold@users.noreply.github.com>

davidhewitt · 2024-03-10T15:38:13Z

Thanks everyone for the reviews! Definitely there's more the docs can have improved, though I think this PR really did shift the bulk of them onwards and upwards 💹

davidhewitt mentioned this pull request Mar 6, 2024

New Bound API implementation tracking issue #3684

Closed

63 tasks

davidhewitt force-pushed the bound-docs branch from fa45912 to 718f887 Compare March 6, 2024 22:51

LilyFirefly reviewed Mar 6, 2024

View reviewed changes

guide/src/python_from_rust/calling-existing-code.md Outdated Show resolved Hide resolved

LilyFirefly requested changes Mar 8, 2024

View reviewed changes

guide/src/python_from_rust.md Outdated Show resolved Hide resolved

guide/src/python_from_rust.md Outdated Show resolved Hide resolved

guide/src/python_from_rust.md Outdated Show resolved Hide resolved

Icxolu reviewed Mar 8, 2024

View reviewed changes

davidhewitt force-pushed the bound-docs branch from 509348e to 0547c63 Compare March 9, 2024 09:14

MarshalX reviewed Mar 10, 2024

View reviewed changes

davidhewitt and others added 10 commits March 10, 2024 08:58

wip bound docs

3d08f89

Update guide/src/python_from_rust/calling-existing-code.md

11de7f0

Co-authored-by: Lily Foote <code@lilyf.org>

continue to move and tidy up

6d4093d

Apply suggestions from code review

ea6101e

Co-authored-by: Lily Foote <code@lilyf.org>

update URL

b009daf

complete python-from-rust.md

718faac

progress on types.md; probably more to go

5a69445

update doctest paths

03ff845

review: Icxolu

4682449

finish updating types.md to Bound API

a6da5cf

davidhewitt force-pushed the bound-docs branch from 0547c63 to d08fad7 Compare March 10, 2024 12:15

davidhewitt force-pushed the bound-docs branch from d08fad7 to a20a769 Compare March 10, 2024 12:35

davidhewitt changed the title ~~docs: WIP update for bound~~ docs: major rewrite for Bound API Mar 10, 2024

update remainder of the guide to Bound API

92ba7f1

davidhewitt force-pushed the bound-docs branch from a20a769 to 92ba7f1 Compare March 10, 2024 13:03

Icxolu reviewed Mar 10, 2024

View reviewed changes

davidhewitt mentioned this pull request Mar 10, 2024

release: 0.21.0-beta.0 #3944

Merged

davidhewitt and others added 3 commits March 10, 2024 14:24

Update guide/src/performance.md

4b8d6cc

Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>

Update guide/src/types.md

16925a1

Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>

Update src/lib.rs

df1ff75

review: Icxolu

403dda4

davidhewitt marked this pull request as ready for review March 10, 2024 14:43

adamreichold reviewed Mar 10, 2024

View reviewed changes

guide/src/python-from-rust.md Outdated Show resolved Hide resolved

adamreichold reviewed Mar 10, 2024

View reviewed changes

guide/src/python-from-rust.md Outdated Show resolved Hide resolved

Update guide/src/python-from-rust.md

b015982

Co-authored-by: Adam Reichold <adamreichold@users.noreply.github.com>

Icxolu approved these changes Mar 10, 2024

View reviewed changes

adamreichold reviewed Mar 10, 2024

View reviewed changes

guide/src/async-await.md Outdated Show resolved Hide resolved

davidhewitt and others added 2 commits March 10, 2024 15:16

Update guide/src/async-await.md

5a41b61

Co-authored-by: Adam Reichold <adamreichold@users.noreply.github.com>

review: adamreichold

676524c

adamreichold approved these changes Mar 10, 2024

View reviewed changes

davidhewitt enabled auto-merge March 10, 2024 15:38

davidhewitt added this pull request to the merge queue Mar 10, 2024

Merged via the queue into PyO3:main with commit 9145fcf Mar 10, 2024

davidhewitt deleted the bound-docs branch March 10, 2024 16:42

	* It provides global APIs for the Python interpreter, such as [`py.eval()`][eval] and [`py.import()`][import].
	* It provides global APIs for the Python interpreter, such as [`py.eval_bound()`][eval] and [`py.import_bound()`][import].


		By having the binding to the `'py` lifetime, `Bound<'py, T>` can offer the complete PyO3 API at maximum efficiency. This means that in almost all cases where `Py<T>` is not necessary for lifetime reasons, `Bound<'py, T>` should be used.

		`Bound<'py, T>` engages in Python reference counting. This means that `Bound<'py, T>` owns a Python object. Rust code which just wants to borrow a Python object should use a shared reference `&Bound<'py, T>`. Just like `std::sync::Arg`, using `.clone()` and `drop()` will cheaply implement and decrement the reference count of the object (just in this case, the reference counting is implemented by the Python interpreter itself).

Conversation

davidhewitt commented Feb 26, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codspeed-hq bot commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #3906 will not alter performance

🎉 Hooray! pytest-codspeed just leveled up to 2.2.0!

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarshalX commented Mar 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidhewitt commented Mar 10, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Icxolu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davidhewitt commented Mar 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codspeed-hq bot commented Mar 8, 2024 •

edited

Loading

🎉 Hooray! `pytest-codspeed` just leveled up to 2.2.0!

MarshalX commented Mar 10, 2024 •

edited

Loading

davidhewitt commented Mar 10, 2024 •

edited

Loading