Add a map method to PyRef & PyRefMut

[peter-formlogic]

I've read through the documentation & haven't seen an easy way to do this, although it might be possible.

Sometimes you want to provide a mutable reference to a field contained within a struct. If you are using std's RefCell and it's equivalent Ref or RefMut, what you can do is call map to provide mutable access to fields within structs.

It would be great if the same thing could happen with PyRefMut & PyRef.

Here's an example of what I am trying to accomplish:

#[pyclass]
struct Example {
    inner: ExampleInner
}

#[pyclass]
struct ExampleInner {
    field: String
}

#[pymethods]
impl Example {
    pub fn inner(mut slf: PyRefMut<'_, Self>) -> PyRefMut<'_, ExampleInner> {
        slf.map(|val| val.inner) // This does not work currently
    }
}

[davidhewitt]

Yes absolutely agree this API makes sense.

I think in combination with the (poorly-worded) idea in #1089 it would hopefully become possible to create Python objects which don't have their own data, instead just borrowing from sub-fields of other objects.

Experimentation in this area is very welcome 👍

[mejrs]

I imagine we would want these as associated methods like std's guards. It would be called like

#[pymethods]
impl Example {
    pub fn inner(mut slf: PyRefMut<'_, Self>) -> PyRefMut<'_, ExampleInner> {
        PyRefMut::map(slf, |val| val.inner)
    }
}

👍 from me

[birkenfeld]

Small correction, needs to be |val| &val.inner|.

[Kobzol]

I tried to play with this. Passing the PyRef by value to map seems a bit problematic:

let python_obj: Py<...> = ...;

let pyref1 = python_obj.as_ref(py).borrow();
let pyref2 = PyRef::map(pyref2, |pyref| pyref.field.as_ref(py));

The problem is that for accessing field, pyref has to go through (auto)deref, which means that the resulting reference is tied to the map argument, but that is passed by value, so it doesn't work, since we would be returning a reference to a temporary value from the map.

I was able to make it work by passing the PyRef by reference, like this:

pub fn map<'s, F, Target>(&'s self, f: F) -> PyRef<'p, Target>
        where F: FnOnce(&'s Self) -> &'p PyCell<Target>,
            Target: PyClass,
    {
        PyRef {
            inner: f(self)
        }
    }

The problem is that with this API, I'm able to return some completely unrelated PyCell from F (one that doesn't come from self) which I'm not sure is OK.

[peter-formlogic]

I think I know what you mean, but let me know if I'm wrong: It's that the map function can return anything, not just something that comes from self, but any captured variable from elsewhere.

In that case, I've had a look at whether that is possible via RefCell & Ref and it seems like it is possible there (with a 'static lifetime):

use std::cell::{RefCell, Ref};

static UNRELATED: usize = 4;

fn main() {

    let c = RefCell::new(5);
    let b1 = c.borrow();
    
    Ref::map(b1, |_| &UNRELATED);

}

I don't know if that is something that needs to be restricted, since it's possible in std, unless there are some unsafe or other considerations.

[Kobzol]

Good point. I tried to make something like this work:

fn map_ref<'a>(py: Python<'a>, pyref: PyRef<'a, S1>) -> PyRef<'a, S2> {
    let inner: &'a PyCell<S2> = pyref.s2.as_ref(py);
    PyRef {
        inner: inner
    }
}

but this won't work, because pyref.s2.as_ref takes the lifetime of pyref instead of 'a, because of the definition of as_ref:

pub fn as_ref<'py>(&'py self, _py: Python<'py>) -> &'py T::AsRefTarget

It takes the shortest of the two lifetimes of self and 'py.

This is done on purpose, there's into_ref which does not have this limitation, but it has some overhead according to the documentation. But I don't think that into_ref is usable here, as it would need to move from its self argument, but we can't move from PyRef.

To sum up, I'm not sure if it's even possible to write the function foo(PyRef<A>) -> PyRef<B> with the current PyRef design. Any ideas?

[jeertmans]

I have tried a bit and the only thing I managed to do is the following:

impl<'p, T: PyClass> PyRef<'p, T> {
    pub fn map<U: PyClass + AsRef<PyAny>, F>(orig: PyRef<'p, T>, f: F) -> PyResult<PyRef<'p, U>>
    where
        F: FnOnce(&T) -> &U,
    {
        unsafe {
            let inner: &'p T = &*orig.inner.get_ptr();
            // Or:
            // let inner: &'p T = orig.inner.try_borrow_unguarded()?;

            let value: &'p U = f(inner);
            PyRef::extract(value.as_ref())
        }
    }
}

Which works but requires the output type to implement AsRef<PyAny>, which is not the case in general for PyClass. Any idea if we can circumvent this problem?

[dimbleby]

"good first issue" seems optimistic

[davidhewitt]

@dimbleby reasonable suggestion. Originally I tagged it because I thought it was an API design problem and relatively orthogonal to the rest of PyO3, but I think now this is relatively coupled to the internal representation of PyO3 types.

[cmpute]

As a related question, does the macro backend generates different output for a pyclass field and a non-pyclass field? Or in the example provided by the OP, it is possible to obtain a Py<ExampleInner> or PyCell<ExampleInner> from an &Example?

[davidhewitt]

At the moment that's not possible, but I'd definitely want it to be. At the moment a Py<ExampleInner> assumes that it owns the Rust data for ExampleInner, but with a rework to PyO3's internals (in particular, the contents of PyCell), I'd like to make it possible for that to just refer back to data owned in Py<Example>. That's what I'm thinking about in #1089.

A possible first step would be to allow this for #[pyclass(frozen)], because that avoids the complexity of concurrent modification. It depends really whether it's easy for us to express that with the current traits or we need a more intensive rethink.

With the introduction of the new Bound API I'd quite like to remove PyCell from the public PyO3 API, and then we can be more free to rethink how that's built internally.