Trouble using PyContextProtocol

[jothan]

I'm trying to make a file writing interface be its own context manager with PyO3 0.12. The trouble is that I don't know how to return self from the __enter__ function. I don't have this issue for implementing iterators that are their own iterators.

I can work around this by not using the context protocol and simply using methods inside the pymethods block.

Code that I'm trying to make work: https://github.com/jothan/cordoba/blob/cf8c6deb809d73948c8c47677c6b92b799588131/src/pymod.rs#L200

Iterator that can return self: https://github.com/jothan/cordoba/blob/cf8c6deb809d73948c8c47677c6b92b799588131/src/pymod.rs#L118

[davidhewitt]

Thanks, yes this is sadly a flaw with the #[pyproto] design at the moment which I've been aware of for a while but not found the right moment to fix. Perhaps this is now a good time. I've opened #1206 and will try to take a spin at this soon.

[kngwyu]

For now, I'm happy to write a temporary solution (i.e. allow PyRefMut) for this.
@davidhewitt What do you think?

[davidhewitt]

I think what I have in mind is non-breaking and not too hard so I can do over the weekend and we can potentially even release in a 0.12.2.

[indygreg]

Fun fact: PyContextProtocol isn't required to implement __enter__ and __exit__. Since these methods don't occupy any special slots on PyTypeObject (from Python's C API), you can implement __enter__ and __exit__ next to your normal methods on a #[pymethods] impl block.

You can return self by doing something like:

#[pymethods]
impl MyClass {
    fn __enter__<'p>(slf: PyRef<'p, Self>, _py: Python<'p>) -> PyResult<PyRef<'p, Self>> {
        Ok(slf)
    }
}

You can also use PyRefMut if you need a mutable instance.

I am curious why PyContextProtocol exists in the first place. I naively assumed the various traits existed to facilitate conversion to the appropriate PyTypeObject slots. But __enter__ and __exit__ are defined on tp_methods and don't require special slot handling. So maybe there's another reason for their existence?

[kngwyu]

@indygreg

So maybe there's another reason for their existence?

No, there's no some complex reason. It's just a convention for users: i.e., we treat __dunder__ methods as protocol, even when they are not actually so in CPython API.

[davidhewitt]

+1 - I'm not aware of the original design choice for #[pyproto]. I think it's because having the trait documentations feels "idiomatic Rust" and it does help to document and group the slot methods.

However I have to confess I've been wondering what happens if we deprecated #[pyproto] in favour of just #[pymethods] (see #1206). It could simplify things. Certainly I'm tempted to experiment in that direction sometime.

[davidhewitt]

As PyContextProtocol is now effectively deprecated, I'm going to close this.

[samuelcolvin]

Is this still the correct way to call __enter__ today?

    pub fn __enter__(slf: PyRef<Self>) -> PyRef<Self> {
        slf
    }

?

I couldn't find any examples in the docs.

[davidhewitt]

Yes, that looks right.

As a tiny optimization, if you don't need to access the class contents, I think you can avoid needing to do the runtime borrow check by using Py<Self>:

    pub fn __enter__(slf: Py<Self>) -> Py<Self> {
        slf
    }

(If that doesn't work, it should be trivial for us to make it work.)

[samuelcolvin]

Thanks so much, seems to work fine, samuelcolvin/watchfiles#164 updated.

[Narsil]

Hey, I've been bitten by the fact that __enter__ and __exit__ don't actually drop the underlying Rust struct.

huggingface/safetensors#164

I'm guessing there's not good solution for it, is there ? (since the context_manager variable seems to outlive the context in Python, so maybe PyO3 cannot actually drop).

I could have made the struct be a wrapper itself, containing Option<Inner> and dropping Inner in __exit__ but that felt quite clunky.

Anything I missed ?

[davidhewitt]

Option<Inner> sounds correct to me; looking at the Python implementation you merged to solve this by assigning a field self.f and have del self.f in the __exit__. The Rust implementation with Option is equivalent, you'd assign self.f = Some(...) in __enter__ and then self.f = None on __exit__.

As you say, the context manager itself can't magically drop itself on __exit__ because the Python reference could live indefinitely long.