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):

#3775

---

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

• [x] Make all of the types and method traits public API
  • [x] ~~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.
• [x] Headline entry in migration guide
• [x] Rename Py2 -> Bound and Py2Borrowed -> Borrowed - #3686
• [x] Finish implementation of all PyXMethods traits for types which have &self methods
  • [x] PyCapsuleMethods - #3770
  • [x] PyComplexMethods - #3767
  • [x] datetime types - #3679
  • [x] PyFrozenSetMethods - #3694
  • [x] PyModuleMethods - #3703
  • [x] PySetMethods - #3694
  • [x] PySliceMethods - #3763
  • [x] PyTracebackMethods - #3763
  • [x] PyTupleMethods - #3681
  • [x] 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 - #3866
  • [x] datetime types - #3778
  • [x] Python (lots of methods)
    • #3806
    • #3816
    • #3832
    • #3846
  • [x] dict constructors - #3823
  • [x] PyEllipsis - #3797
  • [x] PyCFunction / PyFunction - #3800
  • [x] PyNotImplemented - #3797
  • [ ] PyModule constructors - #3775
  • [x] PyType - #3705
  • [x] PyBuffer::get - #3836
  • [x] pyo3::marshal dumps / loads - #3796
  • [x] PyTypeInfo methods - #3782
  • [x] PyErr::from_type, PyErr::value etc. (lots of methods of PyErr need updating)
    • #3819
    • #3820
    • #3826
  • [x] PyUnicodeDecodeError constructors
• [ ] Move FromPyObject to take &Bound<'py, PyAny> instead of &'py PyAny
  • [x] Initial solution proposed in #3706
  • [x] Go through PyO3's codebase and replace fn extract with fn extract_bound for FromPyObject implementations - #3784
  • [ ] FromPyObject for &str, Cow<[u8]> and &[u8] potentially need to be gated behind the gil-refs feature? https://github.com/PyO3/pyo3/pull/3784#discussion_r1477870774
  • [x] #[derive(FromPyObject)] proc macro needs generated output updated - #3828
• [ ] Make Bound<T> the replacement for &PyCell<T>.
  • [x] Add #[pyclass] accessors like .borrow(), borrow_mut() to Bound<'_, T: PyClass> - #3792
  • [ ] Remove &PyCell member from internals of PyRef and PyRefMut - #3860
  • [ ] Remove uses of PyCell from proc macro code (#[pymethods] method receivers)
  • [ ] Deprecate PyCell<T> as a user-facing type?
• [ ] Replacing internal gil-ref uses with new API - some exists in #3606 but not complete
  • Suggestion for people interested in helping with this: pick a file inside src, check for use of GIL Ref APIs. If the functions are internal, rewrite them in the new API. If they are external, consider adding _bound variants of these functions.
  • Good things to search for are .into_gil_ref() and .as_gil_ref() calls, as these are used when escaping from the Bound API into the GIL Refs API. Is the use necessary for compatibility, or just because there's some implementation which hasn't been updated yet?
• [x] ~~Update py.None(), py.Ellipsis() and py.NotImplemented() to return Borrowed<'py, 'py, PyNone> (etc.)~~ For now I just reverted #3578 in #3794, to make this problem go away, will add to the polish list instead.
• [ ] Other proc macro functionality using GIL Refs:
  • [x] #[classmethod] and #[pyfunction(pass_module)] - #3831
  • [ ] Allow #[pymodule] macro to take &Bound<'py, PyModule> as the argument. - @davidhewitt has a draft for this as part of #3744
  • [x] #[pyo3(from_py_with)] on function arguments - #3837
  • [ ] #[pyfunction] argument processing - #3708
• [ ] Further deprecations to nudge users off the GIL Refs API
  • [ ] Deprecate Py::as_ref
  • [ ] Deprecate Py::into_ref - #3867
  • [ ] Deprecate Python::from_owned_ptr methods
  • [ ] Deprecate Python::from_borrowed_ptr methods
  • [ ] Deprecate taking GIL Refs as function arguments, if possible?
  • [x] ~~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>.
  • 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?
• [ ] Check Deref implementation for Bound<T> works for Bound<T: PyClass>?

---

Possible polish to consider later:

• [ ] Split PyModuleMethods to move functionality related to creating / building new modules into a PyModuleBuilder? https://github.com/PyO3/pyo3/pull/3703#issuecomment-1871218780
• [ ] Generic IntoPy<Py<T>> for Bound<'_, T>? https://github.com/PyO3/pyo3/pull/3681#discussion_r1437665719
• [ ] Should Borrowed<'py, 'py, T>::into_gil_ref become public API as well?
• [ ] FromPyObject could gain a lifetime 'a to its argument &'a Bound<'py>, allowing:
  • downcasting the Bound to allow impl FromPyObject for &Bound
  • impl FromPyObject<'a, '_> for &'a str without the pool; as the lifetime of the string borrow depends on the lifetime of the input, not the GIL lifetime
• [ ] Update py.None(), py.Ellipsis() and py.NotImplemented() to return Borrowed<'py, 'py, PyNone> (etc.)
• [ ] From<Bound<$name>> for PyErr as per https://github.com/PyO3/pyo3/pull/3820#discussion_r1493076388

---

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

[snuderl]

I have a PR up for the simple ones (PyEllipsis and NotImplemented). I might try to do PyFunction next, but am also open to suggestions

[davidhewitt]

Next I think I'll look at porting marker::Python

👍 I suspect that Python might be very coupled to the PyTypeMethods and also PyModule::import which haven't yet been finished. Hopefully there are ways to make progress on at least one of them in a manageable fashion!

I might try to do PyFunction next, but am also open to suggestions

PyFunction (and PyCFunction) would be very helpful. I think once they are done we can look at what to do about the wrap_pyfunction! macro. (Either we introduce a wrap_pyfunction_bound! macro or we do similar to what we did with intern! in #3781, might be a judgement call as to what we think makes more sense for users...)

[Icxolu]

👍 I suspect that Python might be very coupled to the PyTypeMethods and also PyModule::import which haven't yet been finished. Hopefully there are ways to make progress on at least one of them in a manageable fashion!

I might skip get_type and import for now, because they are indeed not yet finished/merged. run and eval seem like they can be converted just fine, they are just used in so many places. So maybe we want to split that anyway, so that the diff does not get too big.

[davidhewitt]

Yep, I had tried run and eval as part of #3716. That almost certainly should be split up as it's a painful diff...

[davidhewitt]

One strategy might be to merge one or more of the new _bound variants without deprecating the existing methods. Then it should be possible to pick a single method to deprecate and follow that one through to completion. Will lead to more PRs but in more manageable chunks.

I did that already with #3772

[LilyFoote]

I think PyErr and PyUnicodeDecodeError can be checked off now #3820 is merged.

[davidhewitt]

@Icxolu @LilyFoote we haven't been super great at coordinating work, it seems like we've gotten mostly lucky so far in avoiding duplication. Do you think using something like Gitter or Discord to ping what we're working on would be any easier? #3857

[LilyFoote]

Sounds reasonable to me. I'm already on Discord, but could try Gitter if that's preferred.

[davidhewitt]

Part of the problem with Gitter is that I myself am only sporadically on it, as I only really have time to do user support (which is what that one channel really is) at certain times of day.

[Icxolu]

Yeah, for such short pings that might help. GitHub is not really a good platform to ping what everybody is working on. It just fills the thread with information that is often irrelevant a few hours later. I'm also already on Discord, but also open for other options.

[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 #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:

  • #3708
  • #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 #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 https://github.com/PyO3/pyo3/pull/3897#discussion_r1504721475 - 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.