Class attributes can be mutated from Python code

[mejrs]

The guide states:

Note that unlike class variables defined in Python code, class attributes defined in Rust cannot be mutated at all

But this is in fact possible:

use pyo3::prelude::*;

#[pyclass]
struct MyClass {}

#[pymethods]
impl MyClass {
    #[classattr]
    fn my_attribute() -> String {
        "hello".to_string()
    }
}
fn main() {
    Python::with_gil(|py| {
        let my_class = py.get_type::<MyClass>();

        // All good...
        pyo3::py_run!(py, my_class, "assert my_class.my_attribute == 'hello'");

        // OK
        pyo3::py_run!(py, my_class, "my_class.my_attribute = 'foo'");

        // Also OK
        pyo3::py_run!(py, my_class, "assert my_class.my_attribute == 'foo'");
    });
}

[davidhewitt]

Hmm interesting. I suspect this is a behavioural change we missed when we switched to "heap types" to support abi3 - see https://pyo3.rs/v0.16.4/migration.html#runtime-changes-to-support-the-cpython-limited-api

Type objects are now mutable - Python code can set attributes on them.

I'm unsure if this is something we should aim to fix, or just accept that it's been this way for a year and a half and update documentation?

[mejrs]

I'm unsure if this is something we should aim to fix

I think so yes. I would be really surprised if I wrote:

#[pymethods]
impl MyClass {
    #[classattr]
    fn my_attribute() -> String {
        "hello".to_string()
    }
}

and python code was able to change it. IMO that kind of behaviour should be an opt-in. Maybe something like (to be bikeshedded) #[pyo3(changeable_from_python].

[birkenfeld]

Can you overwrite methods from pymethods by reassigning them on the class? If yes, I don't see why class attrs are different.

[davidhewitt]

For what it's worth, a quick search suggests the type flag we'd want to use to implement this is only present in python 3.10: https://docs.python.org/3/c-api/typeobj.html#Py_TPFLAGS_IMMUTABLETYPE

I think for older versions than 3.10 the only way to protect would be a custom metaclass?

[mejrs]

We could set tp_setattro on the type object to point to a function that just returns an error.

[mejrs]

Can you overwrite methods from pymethods by reassigning them on the class? If yes, I don't see why class attrs are different.

Yes

[davidhewitt]

We could set tp_setattro on the type object to point to a function that just returns an error.

I think not on the type but on the type's type, right? i.e. metaclass 😊

[mejrs]

Right. So this means we effectively have to support metaclasses?

[mejrs]

On Python < 3.10, we can set this inside create_type_object_impl:

unsafe extern "C" fn error_setattrofunc(_slf: *mut ffi::PyObject, _attr: *mut ffi::PyObject, _value: *mut ffi::PyObject) -> c_int {
    let py = Python::assume_gil_acquired();

   PyTypeError::new_err("cannot set '...' attribute of immutable type '...'").restore(py);
    -1
}

(*(*type_object).ob_type).tp_setattro = Some(error_setattrofunc);

I've always thought of native classes being immutable as a natural default and an advantage. I would like to restore this behaviour, if you all agree.

Edit: Or is this mutating type? 🤔

[davidhewitt]

I've always thought of native classes being immutable as a natural default and an advantage. I would like to restore this behaviour, if you all agree.

I agree it would be nice to restore this behaviour, but I think it's potentially hard - AFAIK we can't achieve it for the older Pythons without writing a custom metaclass.

I think writing a custom metaclass isn't fun even in pure-Python. There's also a question of where our custom metaclass would live - would it be global data shared amongst all PyO3 objects? How would it interact if multiple packages with different versions of PyO3 are all in use?

If you're feeling brave, please do stab at this! It might take a few attempts for us to figure out what works nicely.

Edit: Or is this mutating type?

Precisely :). If we had out own metaclass, *(*type_object).ob_type) would be our metaclass instead of type.

[birkenfeld]

I agree it would be nice to restore this behaviour, but I think it's potentially hard - AFAIK we can't achieve it for the older Pythons without writing a custom metaclass.

That, and I also think it's unnecessary. "C extension" classes behaving different from normal classes was always an artifact of the non-heaptype implementation. Monkey-patching methods and attributes is a fact of life in Python, and oftentimes even helpful.

[mejrs]

Monkey-patching methods and attributes is a fact of life in Python, and oftentimes even helpful.

I think that largely depends on perspective. It's certainly nice when you, as a user, just want to make something work in a dirty way. As a library author it's terrifying that people can just reach into your implementation details and change stuff. When cpython did the static-> heap type conversion, the new mutability was considered a problem.

Anyway: I don't think we can do this with limited api. As we wouldn't be able to do it consistently we might as well ditch the metaclass idea and only do it on 3.10+ with the Py_TPFLAGS_IMMUTABLETYPE flag. I still think that is preferable over current, but I'm not a fan of the inconsistency.

Also: this gave a me an idea for a new crate :)

[davidhewitt]

Perhaps we should have a #[pyclass(immutable_type_object)] opt-in? We could then document that It only works properly on 3.10+?

[davidhewitt]

Coming back to this, I think we should just add the opt in and leave the default unchanged. I think doing so is a relatively small macro addition to set the flag, so I'll mark this as a good first issue.