Add PyClass borrow methods to Py

[davidhewitt]

This is a bit of an experimental follow-up to #974 .

As Py::new is more efficient than PyCell::new, maybe we can help encourage use of Py::new in most situations by adding .borrow(), .try_borrow(), .borrow_mut() and .try_borrow_mut() to Py.

It seems quite nice to me and leads to no breaking changes. If others also like this then I'd like to update the guide a bit to recommend use of Py<T> a bit clearer before merging.

[kngwyu]

As Py::new is more efficient than PyCell::new

What I meant is Py::new is efficient when it is used as a return value since mem::forget is called internally.

[davidhewitt]

What I meant is Py::new is efficient when it is used as a return value since mem::forget is called internally.

Ah, ok. Shall I close this PR?

EDIT: I think perhaps there's more cases where Py::new can be more efficient - any time where we hand the value straight to Python without accessing from Rust.

If Py::new is usually more efficient, I think in docs we should encourage always using it instead of PyCell::new

[kngwyu]

At least I don't think we need Py::try_borrow or so since we can get &PyCell by AsPyRef.

[birkenfeld]

:( I would have loved this change; most of the time I can't meaningfully interact with a Py<PyClass> before calling .as_ref().borrow().

[davidhewitt]

:( I would have loved this change; most of the time I can't meaningfully interact with a Py before calling .as_ref().borrow().

Tbh me too which was one of the reasons I proposed these methods :D.

I'd definitely in favour of reopening this PR if @kngwyu is willing to accept these despite the slight duplication they introduce.

[kngwyu]

:( I would have loved this change; most of the time I can't meaningfully interact with a

Thank you for the feedback.
Then I think it's OK to have Py::try_borrow or so, but

• Do we need to use Py instead of PyCell in our test code?

• If Py::new is usually more efficient, I think in docs we should encourage always using it instead of PyCell::new

  So we should also encourage the use of PyObject instead of &PyAny? I think PyCell is sometimes more convenient and suitable for explanation.

[davidhewitt]

Do we need to use Py instead of PyCell in our test code?

I think if we think Py::new is more efficient than PyCell::new, it makes sense to use it in test code too? Except maybe in tests where we're testing PyCell itself.

So we should also encourage the use of PyObject instead of &PyAny?

I think we probably should recommend Py<PyAny> instead of PyObject. Though I agree with you it's not so clear what is correct for the Python-native types like PyList etc, where their constructor functions all return &PyList etc...

I think PyCell is sometimes more convenient and suitable for explanation.

I agree with you that PyCell is sometimes more convenient. I'd like to take a moment longer to think about how to document when to use Py vs PyCell - give me a couple of days to write some documentation to add to this PR...

[kngwyu]

Though I think Py::new could be a bit faster when mem::forget is called, I don't think it's practically meaningful to use Py instead of PyCell at all.
Let's merge only additions of Py::borrow families.

[davidhewitt]

Thanks for the feedback - I have simplified this PR to just add the new methods.

[birkenfeld]

Nice!

[davidhewitt]

Alternatively I can add AsRefTarget = PyCell<Self> bound just to Py<T> implementation, but I think it always applies.

[kngwyu]

👍

[kngwyu]

Thank you!