New Bound API implementation tracking issue

[davidhewitt]

This is a tracking issue for #3382

---

PRs currently waiting for review (all input is greatly appreciated, doesn't need to be an existing maintainer):

#3708

---

TODO (this list is a fairly good coverage of all PRs which occurred for the Bound implementation, but it is probably not complete):

• Make all of the types and method traits public API
  • What to do with constructors e.g. PyTuple::new which return gil refs? We have decided we will introduce new_bound variants of these constructors in 0.21 and let migration be incremental over a few release cycles, eventually changing PyTuple::new etc. in maybe 0.23.
• Headline entry in migration guide
• Rename Py2 -> Bound and Py2Borrowed -> Borrowed - make Bound and Borrowed types public API #3686
• Finish implementation of all PyXMethods traits for types which have &self methods
  • PyCapsuleMethods - implement PyCapsuleMethods #3770
  • PyComplexMethods - implement PyComplexMethods #3767
  • datetime types - implement datetime traits for Bound #3679
  • PyFrozenSetMethods - implement PySetMethods and PyFrozenSetMethods #3694
  • PyModuleMethods - implement PyModuleMethods #3703
  • PySetMethods - implement PySetMethods and PyFrozenSetMethods #3694
  • PySliceMethods - implement PyTracebackMethods and PySliceMethods #3763
  • PyTracebackMethods - implement PyTracebackMethods and PySliceMethods #3763
  • PyTupleMethods - implement PyTupleMethods #3681
  • PyTypeMethods - implement PyTypeMethods #3705
• Add Bound constructor variants to all Python types, and also _bound method variants where applicable (this list is not exhaustive, will add as things are found).
  • PyComplex - port PyComplex::from_complex to Bound API #3866
  • datetime types - add _bound constructors for datetime types #3778
  • Python (lots of methods)
    • port Python::eval to Bound API #3806
    • port Python::run to Bound API #3816
    • port Python::import to Bound API #3832
    • port Python::get_type to Bound API #3846
  • dict constructors - deprecate PyDict::new constructor #3823
  • PyEllipsis - PyEllipsis and PyNotImplemented new get_bound api #3797
  • PyCFunction / PyFunction - PyCFunction bound api #3800
  • PyNotImplemented - PyEllipsis and PyNotImplemented new get_bound api #3797
  • PyModule constructors - add PyModule::new_bound and PyModule::import_bound #3775
  • PyType - implement PyTypeMethods #3705
  • PyBuffer::get - convert PyBuffer to Bound API #3836
  • pyo3::marshal dumps / loads - convert marshal to Bound API #3796
  • PyTypeInfo methods - add bound method variants for PyTypeInfo #3782
  • PyErr::from_type, PyErr::value etc. (lots of methods of PyErr need updating)
    • Implement PyErr::get_type_bound #3819
    • Pyerr value bound #3820
    • Pyerr isinstance #3826
    • port PyErr::from_type to Bound API #3885
  • PyUnicodeDecodeError constructors
• Move FromPyObject to take &Bound<'py, PyAny> instead of &'py PyAny
  • Initial solution proposed in Add extract_bound method to FromPyObject #3706
  • Go through PyO3's codebase and replace fn extract with fn extract_bound for FromPyObject implementations - migrate many FromPyObject implementations to Bound API #3784
  • FromPyObject for &str, Cow<[u8]> and &[u8] - add FromPyObjectBound trait for extracting &str without GIL Refs #3928
  • #[derive(FromPyObject)] proc macro needs generated output updated - update #[derive(FromPyObject)] to use extract_bound #3828
• Make Bound<T> the replacement for &PyCell<T>.
  • Add #[pyclass] accessors like .borrow(), borrow_mut() to Bound<'_, T: PyClass> - pyclass methods for Bound #3792
  • Remove &PyCell member from internals of PyRef and PyRefMut - store Bound<T> inside PyRef and PyRefMut #3860
  • Remove uses of PyCell from proc macro code (#[pymethods] method receivers) - allow Bound<'_, T> in #[pymethods] self position #3896
  • Deprecate PyCell::new - deprecate PyCell::new in favor of Py::new or Bound::new #3872
  • Deprecate PyCell<T> as a user-facing type - deprecate the use of PyCell in favor of Bound and Py #3916
• Replacing internal gil-ref uses with new API - with as_ref, into_ref
  • With deprecation of most ways to produce a gil-ref, this seems good to me now.
• Update py.None(), py.Ellipsis() and py.NotImplemented() to return Borrowed<'py, 'py, PyNone> (etc.) For now I just reverted Change return types of py.None(), py.NotImplemented() and py.Ellipsis() to typed singletons #3578 in Revert "Merge pull request #3578 from davidhewitt/typed-helpers" #3794, to make this problem go away, will add to the polish list instead.
• Other proc macro functionality using GIL Refs:
  • #[classmethod] and #[pyfunction(pass_module)] - support Bound for classmethod and pass_module #3831
  • Allow #[pymodule] macro to take &Bound<'py, PyModule> as the argument. Pymodule bound #3897
  • #[pyo3(from_py_with)] on function arguments - allow from_py_with on function args to take a fn(&Bound) -> PyResult #3837
  • #[pyfunction] argument processing - update extract_argument to use Bound APIs #3708
• Further deprecations to nudge users off the GIL Refs API
  • Deprecate Py::as_ref - deprecate Py::as_ref #3864
  • Deprecate Py::into_ref - Deprecate Py::into_ref #3867
  • Deprecate Python::from_owned_ptr methods - Deprecate py.from_owned_ptr methods #3875
  • Deprecate Python::from_borrowed_ptr methods - deprecate from_borrowed_ptr methods #3915
  • Deprecate taking GIL Refs as function arguments, if possible - deprecate GIL refs in function argument #3847
  • Deprecate &PyModule in #[pymodule] - deprecate &PyModule as #[pymodule] argument type #3936
  • Deprecate &PyModule in #[pyfunction(pass_module)] - deprecate gil-refs in "self" position #3943
  • Deprecate &PyAny in #[pyo3(from_py_with)] - deprecate gil-refs in from_py_with #3967, deprecate gil-refs in from_py_with (Part 2) #3972
  • Deprecate wrap_pyfunction!(foo, py) to wrap_pyfunction_bound!(foo, py) (just for the Python second argument)
  • Decide whether we continue to test this huge volume of deprecated APIs - I think for now I'm not bothering to remove tests which operate on GIL Refs. It's quite possible that when we deprecate the feature we end up with a bit of temporary deprecation, but overall if some of the GIL Ref APIs miss coverage it won't be a huge problem if they are basically all just wrappers around the Bound API.
• Update types.md documentation for Bound<T> - docs: major rewrite for Bound API #3906
  • Mention that cloning Bound<T> is cheap / shallow.
• Final scan over documentation before release to catch as much outdated stuff as possible.
• Expose PyAnyMethods etc traits on a path other than pyo3::prelude - reexport PyAnyMethods and friends from pyo3::types #3895
• Check Deref implementation for Bound<T> works for Bound<T: PyClass> - fix AsRef and Deref impls on Bound<T> #3879

---

Possible polish to consider later: Moved to #3960

---

I'll try to keep this updated as we go, and probably there are items which I've missed right now.

If anyone wants to claim any of these unassigned tasks, please ping here and I'll update this so that we don't race.

[davidhewitt]

What to do with constructors e.g. PyTuple::new which return gil refs?
Add deprecation or other nudge to encourage users to migrate to new API without blocking their compiles.

I'm wondering if we can kill two birds with one stone here:

• change constructors to return bound, e.g. PyTuple::new() -> Bound<'_, PyTuple>, to require users to at least handle this
• deprecate APIs which convert to gil refs: Bound::into_gil_ref, Py::as_ref, Py::into_ref

That should produce a relatively small number of deprecation warnings and compile failures, while making it relatively easy for users to keep going with these APIs for now if they wish.

Then in 0.22 we can get more aggressive about gating the whole lot of gil ref APIs away with a feature etc.

[davidhewitt]

Generally I've put myself down for anything that involves wrangling with the #3606 branch, as I'm quite familiar with what's on there and have been rebasing it fairly regularly to keep up with adjustments that we decide in review. I'll proceed to rebase + split that branch up into PRs so that we can get all that in.

One exception: I'm going to start immediately with a PR to do the renaming of Py2 -> Bound, because that's going to conflict with literally everything else.

[adamreichold]

Replacing internal gil-ref uses with new API (not a blocker for release but highly desirable)

I know it is a lot of additional work, but we might want to treat this as a blocker, so that we are at least compatible with gevent by never touching the pool if the user code does not do it explicitly.

Add deprecation or other nudge to encourage users to migrate to new API without blocking their compiles.

I also think we should definitely put the pool behind a feature, at the minimum to enable internal testing whether so a no-pool build is complete and works which I think would not be possible via deprecation. I am still undecided whether to argue for default-on or default-off though.

[davidhewitt]

I know it is a lot of additional work, but we might want to treat this as a blocker, so that we are at least compatible with gevent by never touching the pool if the user code does not do it explicitly.

Sure thing. TBH I think a very large chunk of that is on #3606 because I worked hard to get pydantic-core off the pool in all benchmarks. It might be that if we add the deprecation to as_ref and into_ref we will flush out a lot of the remaining pieces of this too.

I also think we should definitely put the pool behind a feature, at the minimum to enable internal testing whether so a no-pool build is complete and works which I think would not be possible via deprecation. I am still undecided whether to argue for default-on or default-off though.

I'd slightly prefer default-off, because we only get one time really to ask users to add default-features = false before they probably leave this in their Cargo.toml and forget about it. I also am relatively tempted to not test this feature on our CI (maybe except for lint), to avoid a huge duplication of tests, and it's easier to get away with that if we label this feature as optional and unsafe / unsupported.

[davidhewitt]

Added item about updating py.None() etc.

[davidhewitt]

I've added a section which lists the reviews currently outstanding, to help direct attention towards the PRs which will move this forward.

[davidhewitt]

If reviewers are trying to decide what to look review next, #3776 would be extremely helpful as I think that's the final piece of design decision which would then allow me to finish splitting #3606 into sub-PRs.

[davidhewitt]

@Icxolu @snuderl as there's now three of us implementing pieces off this list, perhaps let's message here before we start implementing things just so that we don't end up duplicating work?

For my side, I have a few PRs still open for review which are listed above. Next time I have a good moment for implementation, I think my attention is best spent finishing #3744. I'd also like to write the update for types.md ASAP.

[davidhewitt]

I also added a bunch of items which I know are still missing under the "Add Bound constructor variants to all Python types" bullet above.

[Icxolu]

Good idea, pyo3::marshal I have ready, I'll open that PR in a moment. Next I think I'll look at porting marker::Python

[LilyFoote]

Sounds to me like Discord is easiest.

[davidhewitt]

👍 https://discord.gg/c4BwayXQ

[davidhewitt]

@LilyFoote @Icxolu, as you two have been helping so much with this implementation (thank you so much, you have really sped things up), I thought I'd just take stock quickly and see what we think there is left remaining before release.

There are a few bullets left unchecked, mostly around internal cleanup and documentation now.

• Most APIs seem migrated, perhaps excluding #[pymodule] and wrap_pyfunction_bound!, which I think we are all agreed upon with Update ModuleInitializer to handle Bound API #3894 and one follow-up but just needs sequencing.

• The main "internal cleanup" remaining is around py.from_borrowed_ptr.

• There are two PRs of mine which are just waiting for me to fixup / respond to review comments:

  • update extract_argument to use Bound APIs #3708
  • store Bound<T> inside PyRef and PyRefMut #3860
    If you want to take either of them over, just ping either here or on Discord, otherwise I'll get to those at some point next week probably.

• There is more documentation needed, which I will take an opening pass at this weekend.

• We have a deprecation warning in deprecate GIL refs in function argument #3847 which I would like to see merged, I think there's only a little bit of cleanup needed which I can do next week or so.

Do you see much else remaining? Do you have items you plan to do, or want to take on from the above list? I'm hopeful we can finally release this within the next two weeks! 🚀

[Icxolu]

I think this sums it up quite nicely. &Bound in #[pymodule] and wrap_pyfunction_bound would probably be nice having for updating examples and documentation, so that we can remove all into/as_gil_ref() and as_borrowed in there.

I've just looked into allowing Bound in #[pymethod] self position, further discouraging use of &PyCell there. Seems like the tests finally pass, so I can push that shortly.

[davidhewitt]

Ok, I got around to updating my PRs.

#3860 should be good to review again.

#3708 is also good to review, but we should find the performance regression is not present after #3860 is merged, if we want to wait for that to go in first.

[davidhewitt]

Off the back of #3897 (comment) - I think we did previously intend to seal the Methods traits, just never got around to it. That would be a good thing to do before we release, I think, as we will struggle to seal them once they are user-accessible.

[davidhewitt]

I just thought of #3928 which might solve the &str question.

[davidhewitt]

This is so close to ready now, thank you so much everyone!

I just added a bunch of new bullets with ideas of further places the proc macro code could emit deprecation warnings. I hope to start taking a look at those after we've pushed a beta release. If anyone else feels like having a play with them, feel free to do so.

Other than that, I'm going to hopefully see #3928 and #3906 done and then get the beta release live.

Hopefully we can get the deprecation warnings solved relatively quickly and then prep the final 0.21 release!

[davidhewitt]

I think with the last couple PRs, this list is now, finally complete. What a huge effort here, thank you everyone!

I've moved the follow-ups to #3960 and I think it makes sense to continue there, this issue is long enough already.