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

docs: Add and fix links in free-threading guide. #4673

Merged
merged 4 commits into from
Nov 4, 2024

Conversation

ngoldbaum
Copy link
Contributor

This is mostly adding links. There are a few small text edits and fixes as well.

@ngoldbaum
Copy link
Contributor Author

I could probably use PYO3_DOCS_URL instead of latest in all the links to the API docs. It looks like there's only one existing link in the guide that does that, maybe I should mass-convert all the links to the API docs to do that, except if they need to point to a specific version like in the migration guide.

@davidhewitt
Copy link
Member

Yes please, I think it would be better to use the links to the exact PyO3 version using PYO3_DOCS_URL. 👍

Other than that, this is a welcome improvement, thanks 🙏

@ngoldbaum
Copy link
Contributor Author

Moved all the /latest/ links to use the PYO3_DOCS_URL pattern. Also rebased and added links to the docs I wrote for OnceExt and OnceLockExt in another PR.

@@ -1064,7 +1064,7 @@ Python::with_gil(|py| {
});
```

Furthermore, `Python::acquire_gil` provides ownership of a `GILGuard` which can be freely stored and passed around. This is usually not helpful as it may keep the lock held for a long time thereby blocking progress in other parts of the program. Due to the generative lifetime attached to the GIL token supplied by `Python::with_gil`, the problem is avoided as the GIL token can only be passed down the call chain. Often, this issue can also be avoided entirely as any GIL-bound reference `&'py PyAny` implies access to a GIL token `Python<'py>` via the [`PyAny::py`](https://docs.rs/pyo3/latest/pyo3/types/struct.PyAny.html#method.py) method.
Furthermore, `Python::acquire_gil` provides ownership of a `GILGuard` which can be freely stored and passed around. This is usually not helpful as it may keep the lock held for a long time thereby blocking progress in other parts of the program. Due to the generative lifetime attached to the GIL token supplied by `Python::with_gil`, the problem is avoided as the GIL token can only be passed down the call chain. Often, this issue can also be avoided entirely as any GIL-bound reference `&'py PyAny` implies access to a GIL token `Python<'py>` via the [`PyAny::py`](https://docs.rs/pyo3/0.22.5/pyo3/types/struct.PyAny.html#method.py) method.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this one is a method that will be removed in 0.23

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Super, please merge after reverting just one piece :)

@@ -39,7 +39,7 @@ struct MyClass {}
struct MyClass {}
```

[params-1]: https://docs.rs/pyo3/latest/pyo3/types/struct.PyAny.html
[params-1]: {{#PYO3_DOCS_URL}}/pyo3/types/struct.PyAny.html
Copy link
Member

Choose a reason for hiding this comment

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

This file is actually reused inside lib.rs so can't have this placeholder :(

Suggested change
[params-1]: {{#PYO3_DOCS_URL}}/pyo3/types/struct.PyAny.html
[params-1]: https://docs.rs/pyo3/latest/pyo3/types/struct.PyAny.html

@ngoldbaum ngoldbaum enabled auto-merge November 4, 2024 20:39
@ngoldbaum ngoldbaum added this pull request to the merge queue Nov 4, 2024
Merged via the queue into PyO3:main with commit d45e0bd Nov 4, 2024
44 checks passed
@ngoldbaum ngoldbaum deleted the doc-xref branch November 4, 2024 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants